Add v1beta1 API with Conversion Webhook and Cert-Manager #140

jkhelil · 2023-09-05T09:55:28Z

Changes

update release.yaml kodata file with v1beta1 apis and conversion sec
update release.yaml kodata file with webhook deployment, service, and serviceaccount
Add new env var USE_MANAGED_WEBHOOK_CERTS that controls whether the operator installs and managed tls for the webhook
ReconcileCertManager installs a secret with public/private key pair for the webhook ssl function
Add cert-manager as dependency on the bundle used by the ci
update doc

Fixes #142

Submitter Checklist

Includes tests if functionality changed/was added
Includes docs if changes are user-facing
Set a kind label on this PR
Release notes block has been filled in, or marked NONE

See the contributor guide
for details on coding conventions, github and prow interactions, and the code review process.

Release Note

- Upgrade Shipwright Build with v1beta1 support (v0.12 release candidate)
- Automatic deployment of cert-manager through OLM API dependency resolution.
- Use cert-manager to manage CA certificates for the new conversion webhook. This can be disabled by setting the `USE_MANAGED_WEBHOOK_CERTS` environment variable to `false` in the operator's subscription.

ACTION REQUIRED: automatic deployment of cert-manager requires an operator in the catalog that provides cert-manager APIs. This may not be guaranteed.

kodata/release.yaml

config/manifests/bases/shipwright-operator.clusterserviceversion.yaml

docs/shipwrightbuild.md

jkhelil · 2023-09-17T08:36:08Z

/assign @adambkaplan
/assign @coreydaley
@adambkaplan @coreydaley can you review please ?

adambkaplan

/approve

This is a good first step at getting the operator to support the beta API and associated conversion webhook. There are a few things that I would like to see before merging:

Squashed commits, with a detailed commit message explaining what was added here.
Add a similarly robust release note in the PR description. This is a significant change and probably needs an "ACTION REQUIRED" note due to the added cert-manager dependency.
Update the README to clarify any prerequisites needed if the operator is installed/managed via OLM. For example, does the admin need to separately install the Tekton and Cert-manager operators?

One other thing I noticed that is a bit outside the scope is that our e2e test only checks that the operator installs the component. It doesn't, say, do a smoke test of running a Shipwright build and making sure it works fully end to end. This should be taken up as a separate item.

README.md

adambkaplan · 2023-09-18T14:56:23Z

bundle/manifests/shipwright-operator.clusterserviceversion.yaml

+    - kind: Certificate
+      name: certificates.cert-manager.io
+      version: v1


Does this mean that a cluster admin also needs to install cert-manager?

Does OLM do this automatically, or will an admin need to do this separately? See https://olm.operatorframework.io/docs/concepts/olm-architecture/dependency-resolution/#declaring-dependencies

Via the bundle we provide, OLM installs automatically the dependencies, tekton and cert-manager

We should release the operator to provide the olm bundle in the operatorhubio

controllers/shipwrightbuild_controller.go

adambkaplan · 2023-09-18T15:14:51Z

pkg/certmanager/cert_manager.go

+		}
+	}
+
+	manifest, err := common.SetupManifestival(client, "certificates.yaml", logger)


Not blocking - noticing that this setup code is being run through every reconcile loop. Follow up PRs can refactor things a bit so we only do this setup once. Perhaps use a separate controller?

pkg/tekton/tekton.go

test/catalog/operatorhubio/shipwright-operator-bundle.json

adambkaplan · 2023-09-18T15:37:23Z

README.md

Do we need to update the README to also recommend Tekton and Cert-Manager be installed?

README updated

openshift-ci · 2023-09-18T15:45:14Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: adambkaplan

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [adambkaplan]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

adambkaplan

One final nit on the README, otherwise looks good.

@shipwright-io/operator-reviewers ptal for lgtm.

adambkaplan · 2023-09-21T14:53:16Z

README.md

+## OLM Dependencies
+When installed via OLM using the provided SHipwright Operator Bundle, the Shipwright operator has two dependencies to:
+- The Tekton operator needed by the Shipright Build Controller
+- The Cert-Manager operator needed in case you delegate to the Shipwright operator the ssl certificates management of the Shipwright Conversion webhook
+The two operators are then installed automatically by OLM.


nit to clean up some of the grammar:

Suggested change

## OLM Dependencies

When installed via OLM using the provided SHipwright Operator Bundle, the Shipwright operator has two dependencies to:

- The Tekton operator needed by the Shipright Build Controller

- The Cert-Manager operator needed in case you delegate to the Shipwright operator the ssl certificates management of the Shipwright Conversion webhook

The two operators are then installed automatically by OLM.

## OLM Dependencies

When installed via OLM using the provided Shipwright Operator Bundle, the Shipwright operator will ask OLM to deploy the following operators:

- The [Tekton operator](https://tekton.dev/docs/operator/) to deploy and manage Tekton Pipelines.

- The [Cert-Manager operator](https://cert-manager.io/docs/installation/operator-lifecycle-manager/) to provision certificates for admission/conversion webhooks.

For this to work, the Shipwright operator must be included in a catalog that includes these other operators.

- Update release.yaml with nightly release - Add cert-manager as shipwright operator dependency - ReconcileCertManager to generate ssl key pair for the webhook - Generate rbac, manifests and bundle - update doc

jkhelil · 2023-09-22T13:34:08Z

@shipwright-io/operator-reviewers for lgtm

qu1queee

/lgtm

openshift-ci bot added the release-note label Sep 5, 2023

pull-request-size bot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Sep 5, 2023

openshift-ci bot requested review from HeavyWombat and SaschaSchwarze0 September 5, 2023 09:55

jkhelil changed the title ~~add support for builds v1beta1 api~~ WIP add support for builds v1beta1 api Sep 5, 2023

openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 5, 2023

adambkaplan reviewed Sep 5, 2023

View reviewed changes

kodata/release.yaml Outdated Show resolved Hide resolved

config/manifests/bases/shipwright-operator.clusterserviceversion.yaml Outdated Show resolved Hide resolved

docs/shipwrightbuild.md Outdated Show resolved Hide resolved

jkhelil force-pushed the support_v1beta1 branch from afca365 to d8d6e8d Compare September 5, 2023 14:56

jkhelil changed the title ~~WIP add support for builds v1beta1 api~~ Add Conversion Webhook with CA Manager Strategies Sep 12, 2023

openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 12, 2023

jkhelil force-pushed the support_v1beta1 branch 8 times, most recently from 2d76647 to badfce7 Compare September 17, 2023 08:21

openshift-ci bot assigned adambkaplan and coreydaley Sep 17, 2023

adambkaplan reviewed Sep 18, 2023

View reviewed changes

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 18, 2023

jkhelil force-pushed the support_v1beta1 branch from badfce7 to 722e1d9 Compare September 19, 2023 09:57

qu1queee mentioned this pull request Sep 20, 2023

[FEATURE] Deploy Beta API with Conversion Webhook #142

Closed

1 task

jkhelil mentioned this pull request Sep 21, 2023

Updating Release Manifest to pre-webhook nightly release #141

Closed

adambkaplan added the kind/feature Categorizes issue or PR as related to a new feature. label Sep 21, 2023

openshift-ci bot added release-note-action-required and removed release-note labels Sep 21, 2023

adambkaplan approved these changes Sep 21, 2023

View reviewed changes

adambkaplan changed the title ~~Add Conversion Webhook with CA Manager Strategies~~ Add v1beta1 API with Conversion Webhook and Cert-Manager Sep 21, 2023

Add support for builds v1beta1 api and conversion webhook

30d77ba

- Update release.yaml with nightly release - Add cert-manager as shipwright operator dependency - ReconcileCertManager to generate ssl key pair for the webhook - Generate rbac, manifests and bundle - update doc

jkhelil force-pushed the support_v1beta1 branch from 722e1d9 to 30d77ba Compare September 22, 2023 06:51

qu1queee self-requested a review September 25, 2023 13:13

qu1queee approved these changes Sep 25, 2023

View reviewed changes

openshift-ci bot assigned qu1queee Sep 25, 2023

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Sep 25, 2023

openshift-merge-robot merged commit f244a06 into shipwright-io:main Sep 25, 2023
3 checks passed

adambkaplan mentioned this pull request Sep 27, 2023

remove mutate_image_container_image reference #127

Closed

SaschaSchwarze0 added this to the release-v0.12.0 milestone Nov 2, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add v1beta1 API with Conversion Webhook and Cert-Manager #140

Add v1beta1 API with Conversion Webhook and Cert-Manager #140

jkhelil commented Sep 5, 2023 •

edited by adambkaplan

Loading

jkhelil commented Sep 17, 2023

adambkaplan left a comment

adambkaplan Sep 18, 2023

jkhelil Sep 19, 2023

jkhelil Sep 19, 2023

adambkaplan Sep 18, 2023

adambkaplan Sep 18, 2023

jkhelil Sep 19, 2023

openshift-ci bot commented Sep 18, 2023

adambkaplan left a comment

adambkaplan Sep 21, 2023

jkhelil Sep 22, 2023

jkhelil commented Sep 22, 2023

qu1queee left a comment

Add v1beta1 API with Conversion Webhook and Cert-Manager #140

Add v1beta1 API with Conversion Webhook and Cert-Manager #140

Conversation

jkhelil commented Sep 5, 2023 • edited by adambkaplan Loading

Changes

Submitter Checklist

Release Note

jkhelil commented Sep 17, 2023

adambkaplan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

openshift-ci bot commented Sep 18, 2023

adambkaplan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jkhelil commented Sep 22, 2023

qu1queee left a comment

Choose a reason for hiding this comment

jkhelil commented Sep 5, 2023 •

edited by adambkaplan

Loading