:seedling: Update containers/image to v5.34.1 by tmshort · Pull Request #1827 · operator-framework/operator-controller

tmshort · 2025-03-03T16:28:26Z

However, due to some incompatibility between controller-gen and
github.com/proglottis/gpgme@v0.1.4, we need to move the kubebuilder
comments so controller-gen doesn't try to access gpgme.

This meant moving the kubebuilder:rbac comments into their own
package, and focusing the webhook generation the just one directory.

Description

Reviewer Checklist

API Go Documentation
Tests: Unit Tests (and E2E Tests, if appropriate)
Comprehensive Commit Messages
Links to related GitHub Issue(s)

netlify · 2025-03-03T16:28:49Z

✅ Deploy Preview for olmv1 ready!

Name	Link
🔨 Latest commit	`22199fb`
🔍 Latest deploy log	https://app.netlify.com/sites/olmv1/deploys/67c96821a4ff190008d562b2
😎 Deploy Preview	https://deploy-preview-1827--olmv1.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

camilamacedo86 · 2025-03-03T18:00:12Z

 	rmdir $(CRD_WORKING_DIR)
 	# Generate the remaining operator-controller manifests
-	$(CONTROLLER_GEN) rbac:roleName=manager-role paths="./internal/operator-controller/..." output:rbac:artifacts:config=$(KUSTOMIZE_OPCON_RBAC_DIR)
+	$(CONTROLLER_GEN) rbac:roleName=manager-role paths="./internal/operator-controller/controllers/..." output:rbac:artifacts:config=$(KUSTOMIZE_OPCON_RBAC_DIR)


The problematic dep is called in the controller :-(
I thought in extra in a utils and then do something like that
it might works

That might just be my crazy idea I mentioned earlier

codecov · 2025-03-03T19:17:52Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 68.52%. Comparing base (fed0ad5) to head (22199fb).
Report is 25 commits behind head on main.

Additional details and impacted files

@@ Coverage Diff @@## main #1827 +/- ##
==========================================
+ Coverage 68.39% 68.52% +0.12% 
==========================================
Files 63 65 +2 Lines 5136 5138 +2 ==========================================
+ Hits 3513 3521 +8 + Misses 1392 1388 -4 + Partials 231 229 -2

Flag	Coverage Δ
e2e	`51.69% <100.00%> (+0.16%)`	⬆️
unit	`56.03% <100.00%> (+0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

tmshort · 2025-03-03T19:21:06Z

Replacement for #1692

joelanford · 2025-03-03T19:23:05Z

I think we need to be going to the controller-tools repo to get this fixed there.

tmshort · 2025-03-03T19:24:56Z

I think we need to be going to the controller-tools repo to get this fixed there.

Problem is that it doesn't seem to be controller-tools, specifically. It works fine locally, this seems to be a problem with gpgme somehow.

perdasilva · 2025-03-04T08:50:23Z

I think we need to be going to the controller-tools repo to get this fixed there.
Problem is that it doesn't seem to be controller-tools, specifically. It works fine locally, this seems to be a problem with gpgme somehow.

Could it be working locally because CGO_ENABLED=1?

The issue is the build tags don't get passed down to controller-gen when its loading the packages (somewhere around here). I've done local test hard-coding the containers_image_openpgp tag here and it seemed to work.

We need a way to configure controller-gen to pass build tags to the loader =S

tmshort · 2025-03-04T14:24:12Z

@perdasilva
At first it was failing for me. After I did a go clean -modcache, it started working. Doing that in CI doesn't seem to work, and it also seems fragile.

To be blunt, this PR does work, because it avoids pulling in gpgme code by isolating the RBAC directives. Admittedly, fixing controler-gen would be great, but how long would that take and when would that happen?

camilamacedo86 · 2025-03-04T14:48:34Z


 // +kubebuilder:webhook:admissionReviewVersions={v1},failurePolicy=Fail,groups=olm.operatorframework.io,mutating=true,name=inject-metadata-name.olm.operatorframework.io,path=/mutate-olm-operatorframework-io-v1-clustercatalog,resources=clustercatalogs,verbs=create;update,versions=v1,sideEffects=None,timeoutSeconds=10

-// +kubebuilder:rbac:groups=olm.operatorframework.io,resources=clustercatalogs,verbs=get;list;watch;patch;update


That is interesting, we fail to get the RBAC but not to generate the webhook config based on the markers?

controller-gen also has a limitation that it cannot use multiple directories (as discovered in my earlier merging of the API directories).
So, I pulled all the RBAC into one place.
The webhook generation still looks at this one directory.

However, due to some incompatibility between controller-gen and github.com/proglottis/gpgme@v0.1.4, we need to move the kubebuilder comments so controller-gen doesn't try to access gpgme. This meant moving the kubebuilder:rbac comments into their own package, and focusing the webhook generation the just one directory. Signed-off-by: Todd Short <tshort@redhat.com>

openshift-merge-robot · 2025-03-07T19:03:45Z

PR needs rebase.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

joelanford · 2025-04-02T00:09:29Z

Done in #1892

tmshort requested a review from a team as a code owner March 3, 2025 16:28

tmshort force-pushed the update-containters branch 7 times, most recently from 3fe8072 to 1d6d327Compare March 3, 2025 17:09

camilamacedo86 reviewed Mar 3, 2025
View reviewed changes

tmshort force-pushed the update-containters branch from f485f89 to 872ec2fCompare March 3, 2025 19:06

tmshort force-pushed the update-containters branch from 872ec2f to 8dcfbc2Compare March 3, 2025 19:17

tmshort force-pushed the update-containters branch from 8dcfbc2 to 4569eadCompare March 4, 2025 14:26

camilamacedo86 reviewed Mar 4, 2025
View reviewed changes

tmshort force-pushed the update-containters branch from 4569ead to 91c16b3Compare March 4, 2025 20:09

perdasilva force-pushed the update-containters branch from 91c16b3 to 22199fbCompare March 6, 2025 09:17

openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 7, 2025

joelanford closed this Apr 2, 2025

tmshort deleted the update-containters branch June 25, 2025 18:14

Navigation Menu

Uh oh!

🌱 Update containers/image to v5.34.1 - #1827

🌱 Update containers/image to v5.34.1#1827
tmshort wants to merge 1 commit into
operator-framework:mainfrom
tmshort:update-containters

tmshort commented Mar 3, 2025•
edited
Loading

Uh oh!

netlifyBot commented Mar 3, 2025•
edited
Loading

Uh oh!

camilamacedo86Mar 3, 2025

Uh oh!

tmshortMar 3, 2025

Uh oh!

codecovBot commented Mar 3, 2025•
edited
Loading

Uh oh!

tmshort commented Mar 3, 2025•
edited
Loading

Uh oh!

joelanford commented Mar 3, 2025

Uh oh!

tmshort commented Mar 3, 2025

Uh oh!

perdasilva commented Mar 4, 2025•
edited
Loading

Uh oh!

tmshort commented Mar 4, 2025

Uh oh!

camilamacedo86Mar 4, 2025

Uh oh!

tmshortMar 4, 2025

Uh oh!

openshift-merge-robot commented Mar 7, 2025

Uh oh!

joelanford commented Apr 2, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants


		// +kubebuilder:webhook:admissionReviewVersions={v1},failurePolicy=Fail,groups=olm.operatorframework.io,mutating=true,name=inject-metadata-name.olm.operatorframework.io,path=/mutate-olm-operatorframework-io-v1-clustercatalog,resources=clustercatalogs,verbs=create;update,versions=v1,sideEffects=None,timeoutSeconds=10

		// +kubebuilder:rbac:groups=olm.operatorframework.io,resources=clustercatalogs,verbs=get;list;watch;patch;update

Uh oh!

Conversation

tmshort commented Mar 3, 2025• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Reviewer Checklist

Uh oh!

netlifyBot commented Mar 3, 2025• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for olmv1 ready!

Uh oh!

camilamacedo86Mar 3, 2025

Choose a reason for hiding this comment

Uh oh!

tmshortMar 3, 2025

Choose a reason for hiding this comment

Uh oh!

codecovBot commented Mar 3, 2025• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

tmshort commented Mar 3, 2025• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

joelanford commented Mar 3, 2025

Uh oh!

tmshort commented Mar 3, 2025

Uh oh!

perdasilva commented Mar 4, 2025• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tmshort commented Mar 4, 2025

Uh oh!

camilamacedo86Mar 4, 2025

Choose a reason for hiding this comment

Uh oh!

tmshortMar 4, 2025

Choose a reason for hiding this comment

Uh oh!

openshift-merge-robot commented Mar 7, 2025

Uh oh!

joelanford commented Apr 2, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

tmshort commented Mar 3, 2025•
edited
Loading

netlifyBot commented Mar 3, 2025•
edited
Loading

codecovBot commented Mar 3, 2025•
edited
Loading

tmshort commented Mar 3, 2025•
edited
Loading

perdasilva commented Mar 4, 2025•
edited
Loading