Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add feature annotation key prefix #431

Closed
wants to merge 2 commits into from

Conversation

alexeykazakov
Copy link
Contributor

Description

A few sentences describing the overall goals of the pull request's commits.

Checks

  1. Did you run make generate target? yes

  2. Did make generate change anything in other projects (host-operator, member-operator)? no

// FeatureAnnotationKeyPrefix is used in feature annotation keys in Space, NSTemplate and other resources
// to refer to the corresponding feature toggle from the configuration:
// "toolchain.dev.openshift.com/feature/<feature-name>
FeatureAnnotationKeyPrefix = FeatureToggleNameAnnotationKey + "/"
Copy link
Contributor

Choose a reason for hiding this comment

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

another option would be to list the enabled features in the value of the annotation:

Suggested change
FeatureAnnotationKeyPrefix = FeatureToggleNameAnnotationKey + "/"
toolchain.dev.openshift.com/features: o-lightspeed,another-one,...

That would be easy to manage too, right? Or is there any specific reason for putting the key into the key part?
I mean, it would be understandable if we wanted to store it as a label and list them, but that's not the case.

Copy link
Contributor

Choose a reason for hiding this comment

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

Am I misunderstanding this or there can be only one slash in the annotation key according to https://kubernetes.io/docs/concepts/overview/working-with-objects/annotations/#syntax-and-character-set ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mfrancisc you are right. It doesn't actually allow more than one slash in the annotation key. It's not actually clear from the documentation but I tried it and it failed the validation.

@MatousJobanek I was thinking about using a list of features in a single annotation instead of separate annotations for each feature but thought that it would be a bit simpler to handle multiple annotations. But since multiple slashes are not allowed anyway I'm going to switch to a single annotation now.

@alexeykazakov
Copy link
Contributor Author

No needed anymore. Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants