Skip to content
This repository was archived by the owner on Nov 22, 2024. It is now read-only.

Encode the protocol version in the CRD#1097

Merged
andreaTP merged 3 commits into
mainfrom
protocol-version-crd
Aug 30, 2021
Merged

Encode the protocol version in the CRD#1097
andreaTP merged 3 commits into
mainfrom
protocol-version-crd

Conversation

@andreaTP

Copy link
Copy Markdown
Contributor

Move the version validation to the CRD from the ConfigMap.

}
.apiextensions()
.v1beta1()
.customResourceDefinitions()

Copy link
Copy Markdown
Contributor

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?

Copy link
Copy Markdown
Contributor

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?

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.

that's a good idea, have to think how to produce it, maybe running the cli from within the cluster and checking permissions.

Comment thread core/cloudflow-crd/src/main/scala/akka/datap/crd/App.scala
@andreaTP andreaTP force-pushed the protocol-version-crd branch from fa0d999 to d264abe Compare August 30, 2021 16:48
val ProtocolVersion = "6"

val SupportedApplicationDescriptorVersion = 6
val ApplicationDescriptorVersion = 6

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

7?

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.

even ApplicationDescriptor ?

Copy link
Copy Markdown
Contributor

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.

@andreaTP andreaTP force-pushed the protocol-version-crd branch from d264abe to 16afffe Compare August 30, 2021 17:14
@andreaTP andreaTP merged commit 972c9ab into main Aug 30, 2021
@andreaTP andreaTP deleted the protocol-version-crd branch August 30, 2021 18:53
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.

2 participants