Skip to content
This repository was archived by the owner on Mar 3, 2025. It is now read-only.

🐛 Fix matchConditions to be compatible with GenerateName#382

Merged
tmshort merged 1 commit intooperator-framework:mainfrom
tmshort:merge-fixups
Sep 6, 2024
Merged

🐛 Fix matchConditions to be compatible with GenerateName#382
tmshort merged 1 commit intooperator-framework:mainfrom
tmshort:merge-fixups

Conversation

@tmshort
Copy link
Copy Markdown
Contributor

@tmshort tmshort commented Sep 6, 2024

This makes sure we have a name for the resource... the only way this could happen is if generateName is in use. In that case, k8s will update the resource with a name, and we'll catch it in the webhook after that happens.

@codecov
Copy link
Copy Markdown

codecov bot commented Sep 6, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 33.95%. Comparing base (6584e7e) to head (49e6d43).
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #382   +/-   ##
=======================================
  Coverage   33.95%   33.95%           
=======================================
  Files          16       16           
  Lines         698      698           
=======================================
  Hits          237      237           
  Misses        435      435           
  Partials       26       26           

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

@tmshort tmshort enabled auto-merge September 6, 2024 18:09
@grokspawn
Copy link
Copy Markdown
Contributor

Is there a way to put some comments around that to make the precedence clear? I initially miscounted the perens and thought that you hadn't contained the existing conditions when adding the new stuff.

Signed-off-by: Todd Short <todd.short@me.com>
@grokspawn
Copy link
Copy Markdown
Contributor

e2e failure looks legit

@tmshort tmshort added this pull request to the merge queue Sep 6, 2024
@tmshort
Copy link
Copy Markdown
Contributor Author

tmshort commented Sep 6, 2024

It seems to be a timing issue, it passed on a re-run.

value:
- name: MissingOrIncorrectMetadataNameLabel
expression: "!has(object.metadata.labels) || !('olm.operatorframework.io/metadata.name' in object.metadata.labels) || object.metadata.labels['olm.operatorframework.io/metadata.name'] != object.metadata.name"
expression: "'name' in object.metadata && (!has(object.metadata.labels) || !('olm.operatorframework.io/metadata.name' in object.metadata.labels) || object.metadata.labels['olm.operatorframework.io/metadata.name'] != object.metadata.name)"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would reinvocationPolicy in the webhook config help?

I assume if the webhook is reinvoked, metadata.name would be available the second time because the name generator is another admission plugin.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, when k8s creates the name, it goes back through the webhook

Merged via the queue into operator-framework:main with commit 40cb322 Sep 6, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants