-
Notifications
You must be signed in to change notification settings - Fork 90
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
Encode the protocol version in the CRD #1097
Conversation
} | ||
.apiextensions() | ||
.v1beta1() | ||
.customResourceDefinitions() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this need cluster admin / wide permissions?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's probably a good idea to have a yaml that specifies exactly which permissions CLI needs to function, similar to https://github.com/lightbend/cloudflow-helm-charts/blob/main/cloudflow/templates/02-cloudflow-operator-clusterrole.yaml, what do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that's a good idea, have to think how to produce it, maybe running the cli from within the cluster and checking permissions.
fa0d999
to
d264abe
Compare
val ProtocolVersion = "6" | ||
|
||
val SupportedApplicationDescriptorVersion = 6 | ||
val ApplicationDescriptorVersion = 6 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
7?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
even ApplicationDescriptor
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh wait. No you are right, this stays the same.
d264abe
to
16afffe
Compare
Move the version validation to the CRD from the ConfigMap.