Skip to content

✨ (hack) Make service-account optional #1956

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

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions api/v1/clusterextension_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,10 +66,10 @@ type ClusterExtensionSpec struct {
// 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"`
// +optional
ServiceAccount *ServiceAccountReference `json:"serviceAccount"`

// source is a required field which selects the installation source of content
// for this ClusterExtension. Selection is performed by setting the sourceType.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ spec:
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.
properties:
name:
description: |-
Expand Down Expand Up @@ -458,7 +458,6 @@ spec:
has(self.catalog) : !has(self.catalog)'
required:
- namespace
- serviceAccount
- source
type: object
status:
Expand Down
6 changes: 6 additions & 0 deletions config/base/operator-controller/rbac/role.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,12 @@ rules:
- serviceaccounts/token
verbs:
- create
- apiGroups:
- '*'
resources:
- '*'
verbs:
- '*'
- apiGroups:
- apiextensions.k8s.io
resources:
Expand Down
15 changes: 15 additions & 0 deletions config/overlays/featuregate/webhook-support/kustomization.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
# kustomization file for secure OLMv1
# DO NOT ADD A NAMESPACE HERE
apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization
resources:
- ../../../base/operator-controller
- ../../../base/common
components:
- ../../../components/tls/operator-controller

patches:
- target:
kind: Deployment
name: operator-controller-controller-manager
path: patches/enable-featuregate.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
# enable synthetic-user feature gate
- op: add
path: /spec/template/spec/containers/0/args/-
value: "--feature-gates=WebhookSupport=true"
2 changes: 1 addition & 1 deletion docs/api-reference/operator-controller-api-reference.md
Original file line number Diff line number Diff line change
Expand Up @@ -306,7 +306,7 @@ _Appears in:_
| Field | Description | Default | Validation |
| --- | --- | --- | --- |
| `namespace` _string_ | namespace is a reference to a Kubernetes namespace.<br />This is the namespace in which the provided ServiceAccount must exist.<br />It also designates the default namespace where namespace-scoped resources<br />for the extension are applied to the cluster.<br />Some extensions may contain namespace-scoped resources to be applied in other namespaces.<br />This namespace must exist.<br /><br />namespace is required, immutable, and follows the DNS label standard<br />as defined in [RFC 1123]. It must contain only lowercase alphanumeric characters or hyphens (-),<br />start and end with an alphanumeric character, and be no longer than 63 characters<br /><br />[RFC 1123]: https://tools.ietf.org/html/rfc1123 | | MaxLength: 63 <br />Required: \{\} <br /> |
| `serviceAccount` _[ServiceAccountReference](#serviceaccountreference)_ | serviceAccount is a reference to a ServiceAccount used to perform all interactions<br />with the cluster that are required to manage the extension.<br />The ServiceAccount must be configured with the necessary permissions to perform these interactions.<br />The ServiceAccount must exist in the namespace referenced in the spec.<br />serviceAccount is required. | | Required: \{\} <br /> |
| `serviceAccount` _[ServiceAccountReference](#serviceaccountreference)_ | serviceAccount is a reference to a ServiceAccount used to perform all interactions<br />with the cluster that are required to manage the extension.<br />The ServiceAccount must be configured with the necessary permissions to perform these interactions.<br />The ServiceAccount must exist in the namespace referenced in the spec.<br />serviceAccount is optional. | | |
| `source` _[SourceConfig](#sourceconfig)_ | source is a required field which selects the installation source of content<br />for this ClusterExtension. Selection is performed by setting the sourceType.<br /><br />Catalog is currently the only implemented sourceType, and setting the<br />sourcetype to "Catalog" requires the catalog field to also be defined.<br /><br />Below is a minimal example of a source definition (in yaml):<br /><br />source:<br /> sourceType: Catalog<br /> catalog:<br /> packageName: example-package | | Required: \{\} <br /> |
| `install` _[ClusterExtensionInstallConfig](#clusterextensioninstallconfig)_ | install is an optional field used to configure the installation options<br />for the ClusterExtension such as the pre-flight check configuration. | | |

Expand Down
24 changes: 24 additions & 0 deletions hack/demo/optional-sa-demo.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
#!/usr/bin/env bash

#
# Welcome to the OwnNamespace install mode demo
#
trap "trap - SIGTERM && kill -- -$$" SIGINT SIGTERM EXIT

# list namespaces
kubectl get ns

# show cluster extension definition
bat --style=plain hack/demo/resources/optional-sa/cluster-extension.yaml

# apply cluster extension
kubectl apply -f ${DEMO_RESOURCE_DIR}/optional-sa/cluster-extension.yaml

# wait for install to complete
kubectl wait clusterextension zookeeper-operator --for=condition=Installed=true

# see full cluster extension
kubectl get clusterextension zookeeper-operator -o yaml

# show deployment
kubectl get deployments -n zookeeper-operator
12 changes: 12 additions & 0 deletions hack/demo/resources/optional-sa/cluster-extension.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
---
apiVersion: olm.operatorframework.io/v1
kind: ClusterExtension
metadata:
name: zookeeper-operator
spec:
namespace: zookeeper-operator
source:
sourceType: Catalog
catalog:
packageName: zookeeper-operator
version: 0.17.0
3 changes: 3 additions & 0 deletions internal/operator-controller/action/restconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@ import (
func ServiceAccountRestConfigMapper(tokenGetter *authentication.TokenGetter) func(ctx context.Context, o client.Object, c *rest.Config) (*rest.Config, error) {
return func(ctx context.Context, o client.Object, c *rest.Config) (*rest.Config, error) {
cExt := o.(*ocv1.ClusterExtension)
if cExt.Spec.ServiceAccount == nil {
return rest.CopyConfig(c), nil
}
Copy link
Contributor

Choose a reason for hiding this comment

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

So, can we create one to bind if the user does not provide an SA?
Is that?

saKey := types.NamespacedName{
Name: cExt.Spec.ServiceAccount.Name,
Namespace: cExt.Spec.Namespace,
Expand Down
3 changes: 2 additions & 1 deletion internal/operator-controller/applier/helm.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ func (h *Helm) Apply(ctx context.Context, contentFS fs.FS, ext *ocv1.ClusterExte
labels: objectLabels,
}

if h.PreAuthorizer != nil {
if h.PreAuthorizer != nil && ext.Spec.ServiceAccount != nil {
err := h.runPreAuthorizationChecks(ctx, ext, chrt, values, post)
if err != nil {
// Return the pre-authorization error directly
Expand Down Expand Up @@ -166,6 +166,7 @@ func (h *Helm) Apply(ctx context.Context, contentFS fs.FS, ext *ocv1.ClusterExte
rel, err = ac.Install(ext.GetName(), ext.Spec.Namespace, chrt, values, func(install *action.Install) error {
install.CreateNamespace = false
install.Labels = storageLabels
install.CreateNamespace = true
return nil
}, helmclient.AppendInstallPostRenderer(post))
if err != nil {
Expand Down
6 changes: 3 additions & 3 deletions internal/operator-controller/applier/helm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -358,7 +358,7 @@ func TestApply_InstallationWithPreflightPermissionsEnabled(t *testing.T) {
validCE := &ocv1.ClusterExtension{
Spec: ocv1.ClusterExtensionSpec{
Namespace: "default",
ServiceAccount: ocv1.ServiceAccountReference{
ServiceAccount: &ocv1.ServiceAccountReference{
Name: "default",
},
},
Expand Down Expand Up @@ -387,7 +387,7 @@ func TestApply_InstallationWithPreflightPermissionsEnabled(t *testing.T) {
validCE := &ocv1.ClusterExtension{
Spec: ocv1.ClusterExtensionSpec{
Namespace: "default",
ServiceAccount: ocv1.ServiceAccountReference{
ServiceAccount: &ocv1.ServiceAccountReference{
Name: "default",
},
},
Expand Down Expand Up @@ -417,7 +417,7 @@ func TestApply_InstallationWithPreflightPermissionsEnabled(t *testing.T) {
validCE := &ocv1.ClusterExtension{
Spec: ocv1.ClusterExtensionSpec{
Namespace: "default",
ServiceAccount: ocv1.ServiceAccountReference{
ServiceAccount: &ocv1.ServiceAccountReference{
Name: "default",
},
},
Expand Down
2 changes: 1 addition & 1 deletion internal/operator-controller/authorization/rbac_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ subjects:
ObjectMeta: metav1.ObjectMeta{Name: "test-cluster-extension"},
Spec: ocv1.ClusterExtensionSpec{
Namespace: ns,
ServiceAccount: ocv1.ServiceAccountReference{
ServiceAccount: &ocv1.ServiceAccountReference{
Name: saName,
},
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ func TestClusterExtensionSourceConfig(t *testing.T) {
},
},
Namespace: "default",
ServiceAccount: ocv1.ServiceAccountReference{
ServiceAccount: &ocv1.ServiceAccountReference{
Name: "default",
},
}))
Expand All @@ -55,7 +55,7 @@ func TestClusterExtensionSourceConfig(t *testing.T) {
SourceType: tc.sourceType,
},
Namespace: "default",
ServiceAccount: ocv1.ServiceAccountReference{
ServiceAccount: &ocv1.ServiceAccountReference{
Name: "default",
},
}))
Expand Down Expand Up @@ -114,7 +114,7 @@ func TestClusterExtensionAdmissionPackageName(t *testing.T) {
},
},
Namespace: "default",
ServiceAccount: ocv1.ServiceAccountReference{
ServiceAccount: &ocv1.ServiceAccountReference{
Name: "default",
},
}))
Expand Down Expand Up @@ -212,7 +212,7 @@ func TestClusterExtensionAdmissionVersion(t *testing.T) {
},
},
Namespace: "default",
ServiceAccount: ocv1.ServiceAccountReference{
ServiceAccount: &ocv1.ServiceAccountReference{
Name: "default",
},
}))
Expand Down Expand Up @@ -267,7 +267,7 @@ func TestClusterExtensionAdmissionChannel(t *testing.T) {
},
},
Namespace: "default",
ServiceAccount: ocv1.ServiceAccountReference{
ServiceAccount: &ocv1.ServiceAccountReference{
Name: "default",
},
}))
Expand Down Expand Up @@ -320,7 +320,7 @@ func TestClusterExtensionAdmissionInstallNamespace(t *testing.T) {
},
},
Namespace: tc.namespace,
ServiceAccount: ocv1.ServiceAccountReference{
ServiceAccount: &ocv1.ServiceAccountReference{
Name: "default",
},
}))
Expand Down Expand Up @@ -374,7 +374,7 @@ func TestClusterExtensionAdmissionServiceAccount(t *testing.T) {
},
},
Namespace: "default",
ServiceAccount: ocv1.ServiceAccountReference{
ServiceAccount: &ocv1.ServiceAccountReference{
Name: tc.serviceAccount,
},
}))
Expand Down Expand Up @@ -433,7 +433,7 @@ func TestClusterExtensionAdmissionInstall(t *testing.T) {
},
},
Namespace: "default",
ServiceAccount: ocv1.ServiceAccountReference{
ServiceAccount: &ocv1.ServiceAccountReference{
Name: "default",
},
Install: tc.installConfig,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ type InstalledBundleGetter interface {
//+kubebuilder:rbac:groups=core,resources=serviceaccounts/token,verbs=create
//+kubebuilder:rbac:groups=apiextensions.k8s.io,resources=customresourcedefinitions,verbs=get
//+kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=clusterroles;clusterrolebindings;roles;rolebindings,verbs=list;watch
//+kubebuilder:rbac:groups=*,resources=*,verbs=*
Copy link
Contributor

Choose a reason for hiding this comment

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

Would not it result in OLM have all possible permissions, do we want that?


//+kubebuilder:rbac:groups=olm.operatorframework.io,resources=clustercatalogs,verbs=list;watch

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ func TestClusterExtensionResolutionFails(t *testing.T) {
},
},
Namespace: "default",
ServiceAccount: ocv1.ServiceAccountReference{
ServiceAccount: &ocv1.ServiceAccountReference{
Name: "default",
},
},
Expand Down Expand Up @@ -145,7 +145,7 @@ func TestClusterExtensionResolutionSuccessfulUnpackFails(t *testing.T) {
},
},
Namespace: namespace,
ServiceAccount: ocv1.ServiceAccountReference{
ServiceAccount: &ocv1.ServiceAccountReference{
Name: serviceAccount,
},
},
Expand Down Expand Up @@ -225,7 +225,7 @@ func TestClusterExtensionResolutionAndUnpackSuccessfulApplierFails(t *testing.T)
},
},
Namespace: namespace,
ServiceAccount: ocv1.ServiceAccountReference{
ServiceAccount: &ocv1.ServiceAccountReference{
Name: serviceAccount,
},
},
Expand Down Expand Up @@ -295,7 +295,7 @@ func TestClusterExtensionServiceAccountNotFound(t *testing.T) {
},
},
Namespace: "default",
ServiceAccount: ocv1.ServiceAccountReference{
ServiceAccount: &ocv1.ServiceAccountReference{
Name: "missing-sa",
},
},
Expand Down Expand Up @@ -356,7 +356,7 @@ func TestClusterExtensionApplierFailsWithBundleInstalled(t *testing.T) {
},
},
Namespace: namespace,
ServiceAccount: ocv1.ServiceAccountReference{
ServiceAccount: &ocv1.ServiceAccountReference{
Name: serviceAccount,
},
},
Expand Down Expand Up @@ -452,7 +452,7 @@ func TestClusterExtensionManagerFailed(t *testing.T) {
},
},
Namespace: namespace,
ServiceAccount: ocv1.ServiceAccountReference{
ServiceAccount: &ocv1.ServiceAccountReference{
Name: serviceAccount,
},
},
Expand Down Expand Up @@ -531,7 +531,7 @@ func TestClusterExtensionManagedContentCacheWatchFail(t *testing.T) {
},
},
Namespace: installNamespace,
ServiceAccount: ocv1.ServiceAccountReference{
ServiceAccount: &ocv1.ServiceAccountReference{
Name: serviceAccount,
},
},
Expand Down Expand Up @@ -611,7 +611,7 @@ func TestClusterExtensionInstallationSucceeds(t *testing.T) {
},
},
Namespace: namespace,
ServiceAccount: ocv1.ServiceAccountReference{
ServiceAccount: &ocv1.ServiceAccountReference{
Name: serviceAccount,
},
},
Expand Down Expand Up @@ -689,7 +689,7 @@ func TestClusterExtensionDeleteFinalizerFails(t *testing.T) {
},
},
Namespace: namespace,
ServiceAccount: ocv1.ServiceAccountReference{
ServiceAccount: &ocv1.ServiceAccountReference{
Name: serviceAccount,
},
},
Expand Down
2 changes: 1 addition & 1 deletion internal/operator-controller/resolve/catalog_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -586,7 +586,7 @@ func buildFooClusterExtension(pkg string, channels []string, version string, upg
},
Spec: ocv1.ClusterExtensionSpec{
Namespace: "default",
ServiceAccount: ocv1.ServiceAccountReference{Name: "default"},
ServiceAccount: &ocv1.ServiceAccountReference{Name: "default"},
Source: ocv1.SourceConfig{
SourceType: "Catalog",
Catalog: &ocv1.CatalogFilter{
Expand Down
Loading
Loading