-
Notifications
You must be signed in to change notification settings - Fork 67
✨ Deprecate spec.ServiceAccount and remove synthetic permissions feature #2242
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -49,8 +49,7 @@ const ( | |
// ClusterExtensionSpec defines the desired state of ClusterExtension | ||
type ClusterExtensionSpec struct { | ||
// namespace is a reference to a Kubernetes namespace. | ||
// This is the namespace in which the provided ServiceAccount must exist. | ||
// It also designates the default namespace where namespace-scoped resources | ||
// It designates the default namespace where namespace-scoped resources | ||
// for the extension are applied to the cluster. | ||
// Some extensions may contain namespace-scoped resources to be applied in other namespaces. | ||
// This namespace must exist. | ||
|
@@ -67,14 +66,13 @@ type ClusterExtensionSpec struct { | |
// +kubebuilder:validation:Required | ||
Namespace string `json:"namespace"` | ||
|
||
// serviceAccount is a reference to a ServiceAccount used to perform all interactions | ||
// Deprecated: ServiceAccount is ignored by OLM and will be removed in a future release. | ||
// serviceAccount was a reference to the ServiceAccount used to perform all interactions | ||
// with the cluster that are required to manage the extension. | ||
// The ServiceAccount must be configured with the necessary permissions to perform these interactions. | ||
// The ServiceAccount must exist in the namespace referenced in the spec. | ||
// serviceAccount is required. | ||
// serviceAccount is optional. | ||
// | ||
// +kubebuilder:validation:Required | ||
ServiceAccount ServiceAccountReference `json:"serviceAccount"` | ||
// +kubebuilder:validation:Optional | ||
ServiceAccount ServiceAccountReference `json:"serviceAccount,omitzero"` | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure what the kubebuilder magic is which would generate the 'deprecated' label for this field, but it would be great to do it, like https://kubernetes.io/blog/2020/09/03/warnings/#custom-resource-definitions. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
|
||
// source is a required field which selects the installation source of content | ||
// for this ClusterExtension. Selection is performed by setting the sourceType. | ||
|
@@ -369,8 +367,9 @@ type CatalogFilter struct { | |
UpgradeConstraintPolicy UpgradeConstraintPolicy `json:"upgradeConstraintPolicy,omitempty"` | ||
} | ||
|
||
// ServiceAccountReference identifies the serviceAccount used fo install a ClusterExtension. | ||
// Deprecated: ServiceAccount is ignored by OLM and will be removed in a future release. | ||
type ServiceAccountReference struct { | ||
// Deprecated: ServiceAccount.Name is ignored by OLM and will be removed in a future release. | ||
// name is a required, immutable reference to the name of the ServiceAccount | ||
// to be used for installation and management of the content for the package | ||
// specified in the packageName field. | ||
|
This file was deleted.
This file was deleted.
This file was deleted.
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.
Would it make sense to also log a warning during reconciliation if this field is still set by a user?
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.
I have some concerns about generating a log on each reconcile. It looks like k8s uses a metric to express "thing is using a deprecated API" and that feels like an approach which could scale better.