From d3a137fc85e12f8fa0ee0ad55a77872a0f21b198 Mon Sep 17 00:00:00 2001
From: Joe Lanford <joe.lanford@gmail.com>
Date: Wed, 26 Feb 2025 15:02:16 -0500
Subject: [PATCH 01/13] auth: use synthetic user/group when service account is
not defined
Signed-off-by: Joe Lanford <joe.lanford@gmail.com>
---
api/v1/clusterextension_types.go | 15 +++++++--
api/v1/zz_generated.deepcopy.go | 6 +++-
cmd/operator-controller/main.go | 2 +-
...peratorframework.io_clusterextensions.yaml | 12 +++++--
.../base/operator-controller/rbac/role.yaml | 7 ++++
config/samples/olm_v1_clusterextension.yaml | 33 +++++++------------
.../operator-controller-api-reference.md | 2 +-
.../operator-controller/action/restconfig.go | 29 ++++++++++++++--
.../authentication/synthetic.go | 27 +++++++++++++++
.../clusterextension_admission_test.go | 25 ++------------
.../clusterextension_controller.go | 1 +
.../clusterextension_controller_test.go | 33 +------------------
.../resolve/catalog_test.go | 3 +-
test/e2e/cluster_extension_install_test.go | 20 +++++------
.../extension_developer_test.go | 17 ++--------
15 files changed, 119 insertions(+), 113 deletions(-)
create mode 100644 internal/operator-controller/authentication/synthetic.go
diff --git a/api/v1/clusterextension_types.go b/api/v1/clusterextension_types.go
index 0141f1a7a..ba83d302e 100644
--- a/api/v1/clusterextension_types.go
+++ b/api/v1/clusterextension_types.go
@@ -66,10 +66,19 @@ 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.
//
- // +kubebuilder:validation:Required
- ServiceAccount ServiceAccountReference `json:"serviceAccount"`
+ // serviceAccount is optional. If a service account is not defined, requests to the apiserver will instead use
+ // username "olmv1:clusterextensions:<clusterExtension.metadata.name>:admin" and groups
+ // "olmv1:clusterextensions:admin" and "system:authenticated"
+ //
+ // Deprecated: Use of serviceAccount is not recommended. Instead, administrators are encouraged
+ // to use the synthetic user/groups described above. All of the same RBAC setup is still required with these
+ // synthetic user/groups. However, this mode is preferred because it requires administrators to specifically
+ // configure RBAC for extension management, rather than enabling piggybacking on existing highly privileged
+ // service accounts that already exist on the cluster.
+ //
+ // +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.
diff --git a/api/v1/zz_generated.deepcopy.go b/api/v1/zz_generated.deepcopy.go
index 37694f61f..45a3e7de2 100644
--- a/api/v1/zz_generated.deepcopy.go
+++ b/api/v1/zz_generated.deepcopy.go
@@ -324,7 +324,11 @@ func (in *ClusterExtensionList) DeepCopyObject() runtime.Object {
// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
func (in *ClusterExtensionSpec) DeepCopyInto(out *ClusterExtensionSpec) {
*out = *in
- out.ServiceAccount = in.ServiceAccount
+ if in.ServiceAccount != nil {
+ in, out := &in.ServiceAccount, &out.ServiceAccount
+ *out = new(ServiceAccountReference)
+ **out = **in
+ }
in.Source.DeepCopyInto(&out.Source)
if in.Install != nil {
in, out := &in.Install, &out.Install
diff --git a/cmd/operator-controller/main.go b/cmd/operator-controller/main.go
index a9cd69863..282cb462f 100644
--- a/cmd/operator-controller/main.go
+++ b/cmd/operator-controller/main.go
@@ -304,7 +304,7 @@ func run() error {
return err
}
tokenGetter := authentication.NewTokenGetter(coreClient, authentication.WithExpirationDuration(1*time.Hour))
- clientRestConfigMapper := action.ServiceAccountRestConfigMapper(tokenGetter)
+ clientRestConfigMapper := action.ClusterExtensionUserRestConfigMapper(tokenGetter)
cfgGetter, err := helmclient.NewActionConfigGetter(mgr.GetConfig(), mgr.GetRESTMapper(),
helmclient.StorageDriverMapper(action.ChunkedStorageDriverMapper(coreClient, mgr.GetAPIReader(), cfg.systemNamespace)),
diff --git a/config/base/operator-controller/crd/bases/olm.operatorframework.io_clusterextensions.yaml b/config/base/operator-controller/crd/bases/olm.operatorframework.io_clusterextensions.yaml
index a582917aa..615973ce9 100644
--- a/config/base/operator-controller/crd/bases/olm.operatorframework.io_clusterextensions.yaml
+++ b/config/base/operator-controller/crd/bases/olm.operatorframework.io_clusterextensions.yaml
@@ -135,7 +135,16 @@ 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. If a service account is not defined, requests to the apiserver will instead use
+ username "olmv1:clusterextensions:<clusterExtension.metadata.name>:admin" and groups
+ "olmv1:clusterextensions:admin" and "system:authenticated"
+
+ Deprecated: Use of serviceAccount is not recommended. Instead, administrators are encouraged
+ to use the synthetic user/groups described above. All of the same RBAC setup is still required with these
+ synthetic user/groups. However, this mode is preferred because it requires administrators to specifically
+ configure RBAC for extension management, rather than enabling piggybacking on existing highly privileged
+ service accounts that already exist on the cluster.
properties:
name:
description: |-
@@ -458,7 +467,6 @@ spec:
has(self.catalog) : !has(self.catalog)'
required:
- namespace
- - serviceAccount
- source
type: object
status:
diff --git a/config/base/operator-controller/rbac/role.yaml b/config/base/operator-controller/rbac/role.yaml
index be89deec1..5d162899b 100644
--- a/config/base/operator-controller/rbac/role.yaml
+++ b/config/base/operator-controller/rbac/role.yaml
@@ -4,6 +4,13 @@ kind: ClusterRole
metadata:
name: manager-role
rules:
+- apiGroups:
+ - ""
+ resources:
+ - groups
+ - users
+ verbs:
+ - impersonate
- apiGroups:
- ""
resources:
diff --git a/config/samples/olm_v1_clusterextension.yaml b/config/samples/olm_v1_clusterextension.yaml
index 4438dfb76..51d4fc50b 100644
--- a/config/samples/olm_v1_clusterextension.yaml
+++ b/config/samples/olm_v1_clusterextension.yaml
@@ -2,13 +2,7 @@
apiVersion: v1
kind: Namespace
metadata:
- name: argocd
----
-apiVersion: v1
-kind: ServiceAccount
-metadata:
- name: argocd-installer
- namespace: argocd
+ name: argocd-system
---
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRoleBinding
@@ -19,9 +13,8 @@ roleRef:
kind: ClusterRole
name: argocd-installer-clusterrole
subjects:
-- kind: ServiceAccount
- name: argocd-installer
- namespace: argocd
+- kind: User
+ name: "olmv1:clusterextensions:argocd-operator:admin"
---
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRole
@@ -76,9 +69,8 @@ roleRef:
kind: ClusterRole
name: argocd-installer-rbac-clusterrole
subjects:
-- kind: ServiceAccount
- name: argocd-installer
- namespace: argocd
+- kind: User
+ name: "olmv1:clusterextensions:argocd-operator:admin"
---
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRole
@@ -222,7 +214,7 @@ apiVersion: rbac.authorization.k8s.io/v1
kind: Role
metadata:
name: argocd-installer-role
- namespace: argocd
+ namespace: argocd-system
rules:
- apiGroups: [""]
resources: [serviceaccounts]
@@ -260,24 +252,21 @@ apiVersion: rbac.authorization.k8s.io/v1
kind: RoleBinding
metadata:
name: argocd-installer-binding
- namespace: argocd
+ namespace: argocd-system
roleRef:
apiGroup: rbac.authorization.k8s.io
kind: Role
name: argocd-installer-role
subjects:
-- kind: ServiceAccount
- name: argocd-installer
- namespace: argocd
+- kind: User
+ name: "olmv1:clusterextensions:argocd-operator:admin"
---
apiVersion: olm.operatorframework.io/v1
kind: ClusterExtension
metadata:
- name: argocd
+ name: argocd-operator
spec:
- namespace: argocd
- serviceAccount:
- name: argocd-installer
+ namespace: argocd-system
source:
sourceType: Catalog
catalog:
diff --git a/docs/api-reference/operator-controller-api-reference.md b/docs/api-reference/operator-controller-api-reference.md
index 84fdbfa64..24dfabf3b 100644
--- a/docs/api-reference/operator-controller-api-reference.md
+++ b/docs/api-reference/operator-controller-api-reference.md
@@ -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 /><br />serviceAccount is optional. If a service account is not defined, requests to the apiserver will instead use<br />username "olmv1:clusterextensions:<clusterExtension.metadata.name>:admin" and groups<br />"olmv1:clusterextensions:admin" and "system:authenticated"<br /><br />Deprecated: Use of serviceAccount is not recommended. Instead, administrators are encouraged<br />to use the synthetic user/groups described above. All of the same RBAC setup is still required with these<br />synthetic user/groups. However, this mode is preferred because it requires administrators to specifically<br />configure RBAC for extension management, rather than enabling piggybacking on existing highly privileged<br />service accounts that already exist on the cluster. | | |
| `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. | | |
diff --git a/internal/operator-controller/action/restconfig.go b/internal/operator-controller/action/restconfig.go
index 6e0121281..cc6816078 100644
--- a/internal/operator-controller/action/restconfig.go
+++ b/internal/operator-controller/action/restconfig.go
@@ -6,17 +6,42 @@ import (
"k8s.io/apimachinery/pkg/types"
"k8s.io/client-go/rest"
+ "k8s.io/client-go/transport"
"sigs.k8s.io/controller-runtime/pkg/client"
ocv1 "github.com/operator-framework/operator-controller/api/v1"
"github.com/operator-framework/operator-controller/internal/operator-controller/authentication"
)
-func ServiceAccountRestConfigMapper(tokenGetter *authentication.TokenGetter) func(ctx context.Context, o client.Object, c *rest.Config) (*rest.Config, error) {
+func ClusterExtensionUserRestConfigMapper(tokenGetter *authentication.TokenGetter) func(ctx context.Context, o client.Object, c *rest.Config) (*rest.Config, error) {
+ saRestConfigMapper := serviceAccountRestConfigMapper(tokenGetter)
+ synthRestConfigMapper := sythenticUserRestConfigMapper()
+
+ return func(ctx context.Context, o client.Object, c *rest.Config) (*rest.Config, error) {
+ cExt := o.(*ocv1.ClusterExtension)
+ if cExt.Spec.ServiceAccount != nil { //nolint:staticcheck
+ return saRestConfigMapper(ctx, o, c)
+ }
+ return synthRestConfigMapper(ctx, o, c)
+ }
+}
+
+func sythenticUserRestConfigMapper() 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)
+ cc := rest.CopyConfig(c)
+ cc.Wrap(func(rt http.RoundTripper) http.RoundTripper {
+ return transport.NewImpersonatingRoundTripper(authentication.SyntheticImpersonationConfig(*cExt), rt)
+ })
+ return cc, nil
+ }
+}
+
+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)
saKey := types.NamespacedName{
- Name: cExt.Spec.ServiceAccount.Name,
+ Name: cExt.Spec.ServiceAccount.Name, //nolint:staticcheck
Namespace: cExt.Spec.Namespace,
}
saConfig := rest.AnonymousClientConfig(c)
diff --git a/internal/operator-controller/authentication/synthetic.go b/internal/operator-controller/authentication/synthetic.go
new file mode 100644
index 000000000..0dbb754e9
--- /dev/null
+++ b/internal/operator-controller/authentication/synthetic.go
@@ -0,0 +1,27 @@
+package authentication
+
+import (
+ "fmt"
+
+ "k8s.io/client-go/transport"
+
+ ocv1 "github.com/operator-framework/operator-controller/api/v1"
+)
+
+func SyntheticUserName(ext ocv1.ClusterExtension) string {
+ return fmt.Sprintf("olmv1:clusterextensions:%s:admin", ext.Name)
+}
+
+func SyntheticGroups(_ ocv1.ClusterExtension) []string {
+ return []string{
+ "olmv1:clusterextensions:admin",
+ "system:authenticated",
+ }
+}
+
+func SyntheticImpersonationConfig(ext ocv1.ClusterExtension) transport.ImpersonationConfig {
+ return transport.ImpersonationConfig{
+ UserName: SyntheticUserName(ext),
+ Groups: SyntheticGroups(ext),
+ }
+}
diff --git a/internal/operator-controller/controllers/clusterextension_admission_test.go b/internal/operator-controller/controllers/clusterextension_admission_test.go
index 3bf58fd48..6ba037ad5 100644
--- a/internal/operator-controller/controllers/clusterextension_admission_test.go
+++ b/internal/operator-controller/controllers/clusterextension_admission_test.go
@@ -44,9 +44,6 @@ func TestClusterExtensionSourceConfig(t *testing.T) {
},
},
Namespace: "default",
- ServiceAccount: ocv1.ServiceAccountReference{
- Name: "default",
- },
}))
}
if tc.unionField == "" {
@@ -55,9 +52,6 @@ func TestClusterExtensionSourceConfig(t *testing.T) {
SourceType: tc.sourceType,
},
Namespace: "default",
- ServiceAccount: ocv1.ServiceAccountReference{
- Name: "default",
- },
}))
}
@@ -114,9 +108,6 @@ func TestClusterExtensionAdmissionPackageName(t *testing.T) {
},
},
Namespace: "default",
- ServiceAccount: ocv1.ServiceAccountReference{
- Name: "default",
- },
}))
if tc.errMsg == "" {
require.NoError(t, err, "unexpected error for package name %q: %w", tc.pkgName, err)
@@ -212,9 +203,6 @@ func TestClusterExtensionAdmissionVersion(t *testing.T) {
},
},
Namespace: "default",
- ServiceAccount: ocv1.ServiceAccountReference{
- Name: "default",
- },
}))
if tc.errMsg == "" {
require.NoError(t, err, "unexpected error for version %q: %w", tc.version, err)
@@ -267,9 +255,6 @@ func TestClusterExtensionAdmissionChannel(t *testing.T) {
},
},
Namespace: "default",
- ServiceAccount: ocv1.ServiceAccountReference{
- Name: "default",
- },
}))
if tc.errMsg == "" {
require.NoError(t, err, "unexpected error for channel %q: %w", tc.channels, err)
@@ -320,9 +305,6 @@ func TestClusterExtensionAdmissionInstallNamespace(t *testing.T) {
},
},
Namespace: tc.namespace,
- ServiceAccount: ocv1.ServiceAccountReference{
- Name: "default",
- },
}))
if tc.errMsg == "" {
require.NoError(t, err, "unexpected error for namespace %q: %w", tc.namespace, err)
@@ -374,7 +356,7 @@ func TestClusterExtensionAdmissionServiceAccount(t *testing.T) {
},
},
Namespace: "default",
- ServiceAccount: ocv1.ServiceAccountReference{
+ ServiceAccount: &ocv1.ServiceAccountReference{
Name: tc.serviceAccount,
},
}))
@@ -433,10 +415,7 @@ func TestClusterExtensionAdmissionInstall(t *testing.T) {
},
},
Namespace: "default",
- ServiceAccount: ocv1.ServiceAccountReference{
- Name: "default",
- },
- Install: tc.installConfig,
+ Install: tc.installConfig,
}))
if tc.errMsg == "" {
require.NoError(t, err, "unexpected error for install configuration %v: %w", tc.installConfig, err)
diff --git a/internal/operator-controller/controllers/clusterextension_controller.go b/internal/operator-controller/controllers/clusterextension_controller.go
index e571174b0..eeaf1c423 100644
--- a/internal/operator-controller/controllers/clusterextension_controller.go
+++ b/internal/operator-controller/controllers/clusterextension_controller.go
@@ -95,6 +95,7 @@ type InstalledBundleGetter interface {
//+kubebuilder:rbac:groups=olm.operatorframework.io,resources=clusterextensions/finalizers,verbs=update
//+kubebuilder:rbac:namespace=system,groups=core,resources=secrets,verbs=create;update;patch;delete;deletecollection;get;list;watch
//+kubebuilder:rbac:groups=core,resources=serviceaccounts/token,verbs=create
+//+kubebuilder:rbac:groups=core,resources=users;groups,verbs=impersonate
//+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
diff --git a/internal/operator-controller/controllers/clusterextension_controller_test.go b/internal/operator-controller/controllers/clusterextension_controller_test.go
index be61891a0..8447c5954 100644
--- a/internal/operator-controller/controllers/clusterextension_controller_test.go
+++ b/internal/operator-controller/controllers/clusterextension_controller_test.go
@@ -69,9 +69,6 @@ func TestClusterExtensionResolutionFails(t *testing.T) {
},
},
Namespace: "default",
- ServiceAccount: ocv1.ServiceAccountReference{
- Name: "default",
- },
},
}
require.NoError(t, cl.Create(ctx, clusterExtension))
@@ -131,7 +128,6 @@ func TestClusterExtensionResolutionSuccessfulUnpackFails(t *testing.T) {
pkgVer := "1.0.0"
pkgChan := "beta"
namespace := fmt.Sprintf("test-ns-%s", rand.String(8))
- serviceAccount := fmt.Sprintf("test-sa-%s", rand.String(8))
clusterExtension := &ocv1.ClusterExtension{
ObjectMeta: metav1.ObjectMeta{Name: extKey.Name},
@@ -145,9 +141,6 @@ func TestClusterExtensionResolutionSuccessfulUnpackFails(t *testing.T) {
},
},
Namespace: namespace,
- ServiceAccount: ocv1.ServiceAccountReference{
- Name: serviceAccount,
- },
},
}
err := cl.Create(ctx, clusterExtension)
@@ -211,7 +204,6 @@ func TestClusterExtensionResolutionAndUnpackSuccessfulApplierFails(t *testing.T)
pkgVer := "1.0.0"
pkgChan := "beta"
namespace := fmt.Sprintf("test-ns-%s", rand.String(8))
- serviceAccount := fmt.Sprintf("test-sa-%s", rand.String(8))
clusterExtension := &ocv1.ClusterExtension{
ObjectMeta: metav1.ObjectMeta{Name: extKey.Name},
@@ -225,9 +217,6 @@ func TestClusterExtensionResolutionAndUnpackSuccessfulApplierFails(t *testing.T)
},
},
Namespace: namespace,
- ServiceAccount: ocv1.ServiceAccountReference{
- Name: serviceAccount,
- },
},
}
err := cl.Create(ctx, clusterExtension)
@@ -295,7 +284,7 @@ func TestClusterExtensionServiceAccountNotFound(t *testing.T) {
},
},
Namespace: "default",
- ServiceAccount: ocv1.ServiceAccountReference{
+ ServiceAccount: &ocv1.ServiceAccountReference{
Name: "missing-sa",
},
},
@@ -342,7 +331,6 @@ func TestClusterExtensionApplierFailsWithBundleInstalled(t *testing.T) {
pkgVer := "1.0.0"
pkgChan := "beta"
namespace := fmt.Sprintf("test-ns-%s", rand.String(8))
- serviceAccount := fmt.Sprintf("test-sa-%s", rand.String(8))
clusterExtension := &ocv1.ClusterExtension{
ObjectMeta: metav1.ObjectMeta{Name: extKey.Name},
@@ -356,9 +344,6 @@ func TestClusterExtensionApplierFailsWithBundleInstalled(t *testing.T) {
},
},
Namespace: namespace,
- ServiceAccount: ocv1.ServiceAccountReference{
- Name: serviceAccount,
- },
},
}
err := cl.Create(ctx, clusterExtension)
@@ -438,7 +423,6 @@ func TestClusterExtensionManagerFailed(t *testing.T) {
pkgVer := "1.0.0"
pkgChan := "beta"
namespace := fmt.Sprintf("test-ns-%s", rand.String(8))
- serviceAccount := fmt.Sprintf("test-sa-%s", rand.String(8))
clusterExtension := &ocv1.ClusterExtension{
ObjectMeta: metav1.ObjectMeta{Name: extKey.Name},
@@ -452,9 +436,6 @@ func TestClusterExtensionManagerFailed(t *testing.T) {
},
},
Namespace: namespace,
- ServiceAccount: ocv1.ServiceAccountReference{
- Name: serviceAccount,
- },
},
}
err := cl.Create(ctx, clusterExtension)
@@ -516,7 +497,6 @@ func TestClusterExtensionManagedContentCacheWatchFail(t *testing.T) {
pkgVer := "1.0.0"
pkgChan := "beta"
installNamespace := fmt.Sprintf("test-ns-%s", rand.String(8))
- serviceAccount := fmt.Sprintf("test-sa-%s", rand.String(8))
clusterExtension := &ocv1.ClusterExtension{
ObjectMeta: metav1.ObjectMeta{Name: extKey.Name},
@@ -531,9 +511,6 @@ func TestClusterExtensionManagedContentCacheWatchFail(t *testing.T) {
},
},
Namespace: installNamespace,
- ServiceAccount: ocv1.ServiceAccountReference{
- Name: serviceAccount,
- },
},
}
err := cl.Create(ctx, clusterExtension)
@@ -597,7 +574,6 @@ func TestClusterExtensionInstallationSucceeds(t *testing.T) {
pkgVer := "1.0.0"
pkgChan := "beta"
namespace := fmt.Sprintf("test-ns-%s", rand.String(8))
- serviceAccount := fmt.Sprintf("test-sa-%s", rand.String(8))
clusterExtension := &ocv1.ClusterExtension{
ObjectMeta: metav1.ObjectMeta{Name: extKey.Name},
@@ -611,9 +587,6 @@ func TestClusterExtensionInstallationSucceeds(t *testing.T) {
},
},
Namespace: namespace,
- ServiceAccount: ocv1.ServiceAccountReference{
- Name: serviceAccount,
- },
},
}
err := cl.Create(ctx, clusterExtension)
@@ -675,7 +648,6 @@ func TestClusterExtensionDeleteFinalizerFails(t *testing.T) {
pkgVer := "1.0.0"
pkgChan := "beta"
namespace := fmt.Sprintf("test-ns-%s", rand.String(8))
- serviceAccount := fmt.Sprintf("test-sa-%s", rand.String(8))
clusterExtension := &ocv1.ClusterExtension{
ObjectMeta: metav1.ObjectMeta{Name: extKey.Name},
@@ -689,9 +661,6 @@ func TestClusterExtensionDeleteFinalizerFails(t *testing.T) {
},
},
Namespace: namespace,
- ServiceAccount: ocv1.ServiceAccountReference{
- Name: serviceAccount,
- },
},
}
err := cl.Create(ctx, clusterExtension)
diff --git a/internal/operator-controller/resolve/catalog_test.go b/internal/operator-controller/resolve/catalog_test.go
index 00467253e..1d28fedcb 100644
--- a/internal/operator-controller/resolve/catalog_test.go
+++ b/internal/operator-controller/resolve/catalog_test.go
@@ -585,8 +585,7 @@ func buildFooClusterExtension(pkg string, channels []string, version string, upg
Name: pkg,
},
Spec: ocv1.ClusterExtensionSpec{
- Namespace: "default",
- ServiceAccount: ocv1.ServiceAccountReference{Name: "default"},
+ Namespace: "default",
Source: ocv1.SourceConfig{
SourceType: "Catalog",
Catalog: &ocv1.CatalogFilter{
diff --git a/test/e2e/cluster_extension_install_test.go b/test/e2e/cluster_extension_install_test.go
index a01124bfb..02d96143d 100644
--- a/test/e2e/cluster_extension_install_test.go
+++ b/test/e2e/cluster_extension_install_test.go
@@ -343,7 +343,7 @@ func TestClusterExtensionInstallRegistry(t *testing.T) {
},
},
Namespace: ns.Name,
- ServiceAccount: ocv1.ServiceAccountReference{
+ ServiceAccount: &ocv1.ServiceAccountReference{
Name: sa.Name,
},
}
@@ -403,7 +403,7 @@ func TestClusterExtensionInstallRegistryDynamic(t *testing.T) {
},
},
Namespace: ns.Name,
- ServiceAccount: ocv1.ServiceAccountReference{
+ ServiceAccount: &ocv1.ServiceAccountReference{
Name: sa.Name,
},
}
@@ -482,7 +482,7 @@ func TestClusterExtensionInstallRegistryMultipleBundles(t *testing.T) {
},
},
Namespace: ns.Name,
- ServiceAccount: ocv1.ServiceAccountReference{
+ ServiceAccount: &ocv1.ServiceAccountReference{
Name: sa.Name,
},
}
@@ -526,7 +526,7 @@ func TestClusterExtensionBlockInstallNonSuccessorVersion(t *testing.T) {
},
},
Namespace: ns.Name,
- ServiceAccount: ocv1.ServiceAccountReference{
+ ServiceAccount: &ocv1.ServiceAccountReference{
Name: sa.Name,
},
}
@@ -588,7 +588,7 @@ func TestClusterExtensionForceInstallNonSuccessorVersion(t *testing.T) {
},
},
Namespace: ns.Name,
- ServiceAccount: ocv1.ServiceAccountReference{
+ ServiceAccount: &ocv1.ServiceAccountReference{
Name: sa.Name,
},
}
@@ -637,7 +637,7 @@ func TestClusterExtensionInstallSuccessorVersion(t *testing.T) {
},
},
Namespace: ns.Name,
- ServiceAccount: ocv1.ServiceAccountReference{
+ ServiceAccount: &ocv1.ServiceAccountReference{
Name: sa.Name,
},
}
@@ -692,7 +692,7 @@ func TestClusterExtensionInstallReResolvesWhenCatalogIsPatched(t *testing.T) {
},
},
Namespace: ns.Name,
- ServiceAccount: ocv1.ServiceAccountReference{
+ ServiceAccount: &ocv1.ServiceAccountReference{
Name: sa.Name,
},
}
@@ -773,7 +773,7 @@ func TestClusterExtensionInstallReResolvesWhenNewCatalog(t *testing.T) {
},
},
Namespace: ns.Name,
- ServiceAccount: ocv1.ServiceAccountReference{
+ ServiceAccount: &ocv1.ServiceAccountReference{
Name: sa.Name,
},
}
@@ -834,7 +834,7 @@ func TestClusterExtensionInstallReResolvesWhenManagedContentChanged(t *testing.T
},
},
Namespace: ns.Name,
- ServiceAccount: ocv1.ServiceAccountReference{
+ ServiceAccount: &ocv1.ServiceAccountReference{
Name: sa.Name,
},
}
@@ -897,7 +897,7 @@ func TestClusterExtensionRecoversFromInitialInstallFailedWhenFailureFixed(t *tes
},
},
Namespace: ns.Name,
- ServiceAccount: ocv1.ServiceAccountReference{
+ ServiceAccount: &ocv1.ServiceAccountReference{
Name: sa.Name,
},
}
diff --git a/test/extension-developer-e2e/extension_developer_test.go b/test/extension-developer-e2e/extension_developer_test.go
index c493887b0..8e9d2fd88 100644
--- a/test/extension-developer-e2e/extension_developer_test.go
+++ b/test/extension-developer-e2e/extension_developer_test.go
@@ -19,6 +19,7 @@ import (
"sigs.k8s.io/controller-runtime/pkg/client"
ocv1 "github.com/operator-framework/operator-controller/api/v1"
+ "github.com/operator-framework/operator-controller/internal/operator-controller/authentication"
)
func TestExtensionDeveloper(t *testing.T) {
@@ -54,14 +55,6 @@ func TestExtensionDeveloper(t *testing.T) {
installNamespace := "default"
- sa := &corev1.ServiceAccount{
- ObjectMeta: metav1.ObjectMeta{
- Name: fmt.Sprintf("serviceaccount-%s", rand.String(8)),
- Namespace: installNamespace,
- },
- }
- require.NoError(t, c.Create(ctx, sa))
-
clusterExtension := &ocv1.ClusterExtension{
ObjectMeta: metav1.ObjectMeta{
Name: "registryv1",
@@ -74,9 +67,6 @@ func TestExtensionDeveloper(t *testing.T) {
},
},
Namespace: installNamespace,
- ServiceAccount: ocv1.ServiceAccountReference{
- Name: sa.Name,
- },
},
}
@@ -182,9 +172,8 @@ func TestExtensionDeveloper(t *testing.T) {
},
Subjects: []rbacv1.Subject{
{
- Kind: "ServiceAccount",
- Name: sa.Name,
- Namespace: sa.Namespace,
+ Kind: "User",
+ Name: authentication.SyntheticUserName(*clusterExtension),
},
},
RoleRef: rbacv1.RoleRef{
From d0565844776322a637276bbddf901b9cc9710d09 Mon Sep 17 00:00:00 2001
From: Per Goncalves da Silva <pegoncal@redhat.com>
Date: Thu, 27 Feb 2025 09:02:10 +0100
Subject: [PATCH 02/13] Revert API changes
Signed-off-by: Per Goncalves da Silva <pegoncal@redhat.com>
---
api/v1/clusterextension_types.go | 15 ++-------
api/v1/zz_generated.deepcopy.go | 6 +---
...peratorframework.io_clusterextensions.yaml | 12 ++-----
.../base/operator-controller/rbac/role.yaml | 7 ----
config/samples/olm_v1_clusterextension.yaml | 33 ++++++++++++-------
.../operator-controller-api-reference.md | 2 +-
.../clusterextension_admission_test.go | 25 ++++++++++++--
.../clusterextension_controller.go | 1 -
.../clusterextension_controller_test.go | 33 ++++++++++++++++++-
.../resolve/catalog_test.go | 3 +-
test/e2e/cluster_extension_install_test.go | 20 +++++------
.../extension_developer_test.go | 17 ++++++++--
12 files changed, 110 insertions(+), 64 deletions(-)
diff --git a/api/v1/clusterextension_types.go b/api/v1/clusterextension_types.go
index ba83d302e..0141f1a7a 100644
--- a/api/v1/clusterextension_types.go
+++ b/api/v1/clusterextension_types.go
@@ -66,19 +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. If a service account is not defined, requests to the apiserver will instead use
- // username "olmv1:clusterextensions:<clusterExtension.metadata.name>:admin" and groups
- // "olmv1:clusterextensions:admin" and "system:authenticated"
- //
- // Deprecated: Use of serviceAccount is not recommended. Instead, administrators are encouraged
- // to use the synthetic user/groups described above. All of the same RBAC setup is still required with these
- // synthetic user/groups. However, this mode is preferred because it requires administrators to specifically
- // configure RBAC for extension management, rather than enabling piggybacking on existing highly privileged
- // service accounts that already exist on the cluster.
- //
- // +optional
- ServiceAccount *ServiceAccountReference `json:"serviceAccount"`
+ // +kubebuilder:validation:Required
+ 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.
diff --git a/api/v1/zz_generated.deepcopy.go b/api/v1/zz_generated.deepcopy.go
index 45a3e7de2..37694f61f 100644
--- a/api/v1/zz_generated.deepcopy.go
+++ b/api/v1/zz_generated.deepcopy.go
@@ -324,11 +324,7 @@ func (in *ClusterExtensionList) DeepCopyObject() runtime.Object {
// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
func (in *ClusterExtensionSpec) DeepCopyInto(out *ClusterExtensionSpec) {
*out = *in
- if in.ServiceAccount != nil {
- in, out := &in.ServiceAccount, &out.ServiceAccount
- *out = new(ServiceAccountReference)
- **out = **in
- }
+ out.ServiceAccount = in.ServiceAccount
in.Source.DeepCopyInto(&out.Source)
if in.Install != nil {
in, out := &in.Install, &out.Install
diff --git a/config/base/operator-controller/crd/bases/olm.operatorframework.io_clusterextensions.yaml b/config/base/operator-controller/crd/bases/olm.operatorframework.io_clusterextensions.yaml
index 615973ce9..a582917aa 100644
--- a/config/base/operator-controller/crd/bases/olm.operatorframework.io_clusterextensions.yaml
+++ b/config/base/operator-controller/crd/bases/olm.operatorframework.io_clusterextensions.yaml
@@ -135,16 +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 optional. If a service account is not defined, requests to the apiserver will instead use
- username "olmv1:clusterextensions:<clusterExtension.metadata.name>:admin" and groups
- "olmv1:clusterextensions:admin" and "system:authenticated"
-
- Deprecated: Use of serviceAccount is not recommended. Instead, administrators are encouraged
- to use the synthetic user/groups described above. All of the same RBAC setup is still required with these
- synthetic user/groups. However, this mode is preferred because it requires administrators to specifically
- configure RBAC for extension management, rather than enabling piggybacking on existing highly privileged
- service accounts that already exist on the cluster.
+ serviceAccount is required.
properties:
name:
description: |-
@@ -467,6 +458,7 @@ spec:
has(self.catalog) : !has(self.catalog)'
required:
- namespace
+ - serviceAccount
- source
type: object
status:
diff --git a/config/base/operator-controller/rbac/role.yaml b/config/base/operator-controller/rbac/role.yaml
index 5d162899b..be89deec1 100644
--- a/config/base/operator-controller/rbac/role.yaml
+++ b/config/base/operator-controller/rbac/role.yaml
@@ -4,13 +4,6 @@ kind: ClusterRole
metadata:
name: manager-role
rules:
-- apiGroups:
- - ""
- resources:
- - groups
- - users
- verbs:
- - impersonate
- apiGroups:
- ""
resources:
diff --git a/config/samples/olm_v1_clusterextension.yaml b/config/samples/olm_v1_clusterextension.yaml
index 51d4fc50b..4438dfb76 100644
--- a/config/samples/olm_v1_clusterextension.yaml
+++ b/config/samples/olm_v1_clusterextension.yaml
@@ -2,7 +2,13 @@
apiVersion: v1
kind: Namespace
metadata:
- name: argocd-system
+ name: argocd
+---
+apiVersion: v1
+kind: ServiceAccount
+metadata:
+ name: argocd-installer
+ namespace: argocd
---
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRoleBinding
@@ -13,8 +19,9 @@ roleRef:
kind: ClusterRole
name: argocd-installer-clusterrole
subjects:
-- kind: User
- name: "olmv1:clusterextensions:argocd-operator:admin"
+- kind: ServiceAccount
+ name: argocd-installer
+ namespace: argocd
---
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRole
@@ -69,8 +76,9 @@ roleRef:
kind: ClusterRole
name: argocd-installer-rbac-clusterrole
subjects:
-- kind: User
- name: "olmv1:clusterextensions:argocd-operator:admin"
+- kind: ServiceAccount
+ name: argocd-installer
+ namespace: argocd
---
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRole
@@ -214,7 +222,7 @@ apiVersion: rbac.authorization.k8s.io/v1
kind: Role
metadata:
name: argocd-installer-role
- namespace: argocd-system
+ namespace: argocd
rules:
- apiGroups: [""]
resources: [serviceaccounts]
@@ -252,21 +260,24 @@ apiVersion: rbac.authorization.k8s.io/v1
kind: RoleBinding
metadata:
name: argocd-installer-binding
- namespace: argocd-system
+ namespace: argocd
roleRef:
apiGroup: rbac.authorization.k8s.io
kind: Role
name: argocd-installer-role
subjects:
-- kind: User
- name: "olmv1:clusterextensions:argocd-operator:admin"
+- kind: ServiceAccount
+ name: argocd-installer
+ namespace: argocd
---
apiVersion: olm.operatorframework.io/v1
kind: ClusterExtension
metadata:
- name: argocd-operator
+ name: argocd
spec:
- namespace: argocd-system
+ namespace: argocd
+ serviceAccount:
+ name: argocd-installer
source:
sourceType: Catalog
catalog:
diff --git a/docs/api-reference/operator-controller-api-reference.md b/docs/api-reference/operator-controller-api-reference.md
index 24dfabf3b..84fdbfa64 100644
--- a/docs/api-reference/operator-controller-api-reference.md
+++ b/docs/api-reference/operator-controller-api-reference.md
@@ -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 /><br />serviceAccount is optional. If a service account is not defined, requests to the apiserver will instead use<br />username "olmv1:clusterextensions:<clusterExtension.metadata.name>:admin" and groups<br />"olmv1:clusterextensions:admin" and "system:authenticated"<br /><br />Deprecated: Use of serviceAccount is not recommended. Instead, administrators are encouraged<br />to use the synthetic user/groups described above. All of the same RBAC setup is still required with these<br />synthetic user/groups. However, this mode is preferred because it requires administrators to specifically<br />configure RBAC for extension management, rather than enabling piggybacking on existing highly privileged<br />service accounts that already exist on the cluster. | | |
+| `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 /> |
| `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. | | |
diff --git a/internal/operator-controller/controllers/clusterextension_admission_test.go b/internal/operator-controller/controllers/clusterextension_admission_test.go
index 6ba037ad5..3bf58fd48 100644
--- a/internal/operator-controller/controllers/clusterextension_admission_test.go
+++ b/internal/operator-controller/controllers/clusterextension_admission_test.go
@@ -44,6 +44,9 @@ func TestClusterExtensionSourceConfig(t *testing.T) {
},
},
Namespace: "default",
+ ServiceAccount: ocv1.ServiceAccountReference{
+ Name: "default",
+ },
}))
}
if tc.unionField == "" {
@@ -52,6 +55,9 @@ func TestClusterExtensionSourceConfig(t *testing.T) {
SourceType: tc.sourceType,
},
Namespace: "default",
+ ServiceAccount: ocv1.ServiceAccountReference{
+ Name: "default",
+ },
}))
}
@@ -108,6 +114,9 @@ func TestClusterExtensionAdmissionPackageName(t *testing.T) {
},
},
Namespace: "default",
+ ServiceAccount: ocv1.ServiceAccountReference{
+ Name: "default",
+ },
}))
if tc.errMsg == "" {
require.NoError(t, err, "unexpected error for package name %q: %w", tc.pkgName, err)
@@ -203,6 +212,9 @@ func TestClusterExtensionAdmissionVersion(t *testing.T) {
},
},
Namespace: "default",
+ ServiceAccount: ocv1.ServiceAccountReference{
+ Name: "default",
+ },
}))
if tc.errMsg == "" {
require.NoError(t, err, "unexpected error for version %q: %w", tc.version, err)
@@ -255,6 +267,9 @@ func TestClusterExtensionAdmissionChannel(t *testing.T) {
},
},
Namespace: "default",
+ ServiceAccount: ocv1.ServiceAccountReference{
+ Name: "default",
+ },
}))
if tc.errMsg == "" {
require.NoError(t, err, "unexpected error for channel %q: %w", tc.channels, err)
@@ -305,6 +320,9 @@ func TestClusterExtensionAdmissionInstallNamespace(t *testing.T) {
},
},
Namespace: tc.namespace,
+ ServiceAccount: ocv1.ServiceAccountReference{
+ Name: "default",
+ },
}))
if tc.errMsg == "" {
require.NoError(t, err, "unexpected error for namespace %q: %w", tc.namespace, err)
@@ -356,7 +374,7 @@ func TestClusterExtensionAdmissionServiceAccount(t *testing.T) {
},
},
Namespace: "default",
- ServiceAccount: &ocv1.ServiceAccountReference{
+ ServiceAccount: ocv1.ServiceAccountReference{
Name: tc.serviceAccount,
},
}))
@@ -415,7 +433,10 @@ func TestClusterExtensionAdmissionInstall(t *testing.T) {
},
},
Namespace: "default",
- Install: tc.installConfig,
+ ServiceAccount: ocv1.ServiceAccountReference{
+ Name: "default",
+ },
+ Install: tc.installConfig,
}))
if tc.errMsg == "" {
require.NoError(t, err, "unexpected error for install configuration %v: %w", tc.installConfig, err)
diff --git a/internal/operator-controller/controllers/clusterextension_controller.go b/internal/operator-controller/controllers/clusterextension_controller.go
index eeaf1c423..e571174b0 100644
--- a/internal/operator-controller/controllers/clusterextension_controller.go
+++ b/internal/operator-controller/controllers/clusterextension_controller.go
@@ -95,7 +95,6 @@ type InstalledBundleGetter interface {
//+kubebuilder:rbac:groups=olm.operatorframework.io,resources=clusterextensions/finalizers,verbs=update
//+kubebuilder:rbac:namespace=system,groups=core,resources=secrets,verbs=create;update;patch;delete;deletecollection;get;list;watch
//+kubebuilder:rbac:groups=core,resources=serviceaccounts/token,verbs=create
-//+kubebuilder:rbac:groups=core,resources=users;groups,verbs=impersonate
//+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
diff --git a/internal/operator-controller/controllers/clusterextension_controller_test.go b/internal/operator-controller/controllers/clusterextension_controller_test.go
index 8447c5954..be61891a0 100644
--- a/internal/operator-controller/controllers/clusterextension_controller_test.go
+++ b/internal/operator-controller/controllers/clusterextension_controller_test.go
@@ -69,6 +69,9 @@ func TestClusterExtensionResolutionFails(t *testing.T) {
},
},
Namespace: "default",
+ ServiceAccount: ocv1.ServiceAccountReference{
+ Name: "default",
+ },
},
}
require.NoError(t, cl.Create(ctx, clusterExtension))
@@ -128,6 +131,7 @@ func TestClusterExtensionResolutionSuccessfulUnpackFails(t *testing.T) {
pkgVer := "1.0.0"
pkgChan := "beta"
namespace := fmt.Sprintf("test-ns-%s", rand.String(8))
+ serviceAccount := fmt.Sprintf("test-sa-%s", rand.String(8))
clusterExtension := &ocv1.ClusterExtension{
ObjectMeta: metav1.ObjectMeta{Name: extKey.Name},
@@ -141,6 +145,9 @@ func TestClusterExtensionResolutionSuccessfulUnpackFails(t *testing.T) {
},
},
Namespace: namespace,
+ ServiceAccount: ocv1.ServiceAccountReference{
+ Name: serviceAccount,
+ },
},
}
err := cl.Create(ctx, clusterExtension)
@@ -204,6 +211,7 @@ func TestClusterExtensionResolutionAndUnpackSuccessfulApplierFails(t *testing.T)
pkgVer := "1.0.0"
pkgChan := "beta"
namespace := fmt.Sprintf("test-ns-%s", rand.String(8))
+ serviceAccount := fmt.Sprintf("test-sa-%s", rand.String(8))
clusterExtension := &ocv1.ClusterExtension{
ObjectMeta: metav1.ObjectMeta{Name: extKey.Name},
@@ -217,6 +225,9 @@ func TestClusterExtensionResolutionAndUnpackSuccessfulApplierFails(t *testing.T)
},
},
Namespace: namespace,
+ ServiceAccount: ocv1.ServiceAccountReference{
+ Name: serviceAccount,
+ },
},
}
err := cl.Create(ctx, clusterExtension)
@@ -284,7 +295,7 @@ func TestClusterExtensionServiceAccountNotFound(t *testing.T) {
},
},
Namespace: "default",
- ServiceAccount: &ocv1.ServiceAccountReference{
+ ServiceAccount: ocv1.ServiceAccountReference{
Name: "missing-sa",
},
},
@@ -331,6 +342,7 @@ func TestClusterExtensionApplierFailsWithBundleInstalled(t *testing.T) {
pkgVer := "1.0.0"
pkgChan := "beta"
namespace := fmt.Sprintf("test-ns-%s", rand.String(8))
+ serviceAccount := fmt.Sprintf("test-sa-%s", rand.String(8))
clusterExtension := &ocv1.ClusterExtension{
ObjectMeta: metav1.ObjectMeta{Name: extKey.Name},
@@ -344,6 +356,9 @@ func TestClusterExtensionApplierFailsWithBundleInstalled(t *testing.T) {
},
},
Namespace: namespace,
+ ServiceAccount: ocv1.ServiceAccountReference{
+ Name: serviceAccount,
+ },
},
}
err := cl.Create(ctx, clusterExtension)
@@ -423,6 +438,7 @@ func TestClusterExtensionManagerFailed(t *testing.T) {
pkgVer := "1.0.0"
pkgChan := "beta"
namespace := fmt.Sprintf("test-ns-%s", rand.String(8))
+ serviceAccount := fmt.Sprintf("test-sa-%s", rand.String(8))
clusterExtension := &ocv1.ClusterExtension{
ObjectMeta: metav1.ObjectMeta{Name: extKey.Name},
@@ -436,6 +452,9 @@ func TestClusterExtensionManagerFailed(t *testing.T) {
},
},
Namespace: namespace,
+ ServiceAccount: ocv1.ServiceAccountReference{
+ Name: serviceAccount,
+ },
},
}
err := cl.Create(ctx, clusterExtension)
@@ -497,6 +516,7 @@ func TestClusterExtensionManagedContentCacheWatchFail(t *testing.T) {
pkgVer := "1.0.0"
pkgChan := "beta"
installNamespace := fmt.Sprintf("test-ns-%s", rand.String(8))
+ serviceAccount := fmt.Sprintf("test-sa-%s", rand.String(8))
clusterExtension := &ocv1.ClusterExtension{
ObjectMeta: metav1.ObjectMeta{Name: extKey.Name},
@@ -511,6 +531,9 @@ func TestClusterExtensionManagedContentCacheWatchFail(t *testing.T) {
},
},
Namespace: installNamespace,
+ ServiceAccount: ocv1.ServiceAccountReference{
+ Name: serviceAccount,
+ },
},
}
err := cl.Create(ctx, clusterExtension)
@@ -574,6 +597,7 @@ func TestClusterExtensionInstallationSucceeds(t *testing.T) {
pkgVer := "1.0.0"
pkgChan := "beta"
namespace := fmt.Sprintf("test-ns-%s", rand.String(8))
+ serviceAccount := fmt.Sprintf("test-sa-%s", rand.String(8))
clusterExtension := &ocv1.ClusterExtension{
ObjectMeta: metav1.ObjectMeta{Name: extKey.Name},
@@ -587,6 +611,9 @@ func TestClusterExtensionInstallationSucceeds(t *testing.T) {
},
},
Namespace: namespace,
+ ServiceAccount: ocv1.ServiceAccountReference{
+ Name: serviceAccount,
+ },
},
}
err := cl.Create(ctx, clusterExtension)
@@ -648,6 +675,7 @@ func TestClusterExtensionDeleteFinalizerFails(t *testing.T) {
pkgVer := "1.0.0"
pkgChan := "beta"
namespace := fmt.Sprintf("test-ns-%s", rand.String(8))
+ serviceAccount := fmt.Sprintf("test-sa-%s", rand.String(8))
clusterExtension := &ocv1.ClusterExtension{
ObjectMeta: metav1.ObjectMeta{Name: extKey.Name},
@@ -661,6 +689,9 @@ func TestClusterExtensionDeleteFinalizerFails(t *testing.T) {
},
},
Namespace: namespace,
+ ServiceAccount: ocv1.ServiceAccountReference{
+ Name: serviceAccount,
+ },
},
}
err := cl.Create(ctx, clusterExtension)
diff --git a/internal/operator-controller/resolve/catalog_test.go b/internal/operator-controller/resolve/catalog_test.go
index 1d28fedcb..00467253e 100644
--- a/internal/operator-controller/resolve/catalog_test.go
+++ b/internal/operator-controller/resolve/catalog_test.go
@@ -585,7 +585,8 @@ func buildFooClusterExtension(pkg string, channels []string, version string, upg
Name: pkg,
},
Spec: ocv1.ClusterExtensionSpec{
- Namespace: "default",
+ Namespace: "default",
+ ServiceAccount: ocv1.ServiceAccountReference{Name: "default"},
Source: ocv1.SourceConfig{
SourceType: "Catalog",
Catalog: &ocv1.CatalogFilter{
diff --git a/test/e2e/cluster_extension_install_test.go b/test/e2e/cluster_extension_install_test.go
index 02d96143d..a01124bfb 100644
--- a/test/e2e/cluster_extension_install_test.go
+++ b/test/e2e/cluster_extension_install_test.go
@@ -343,7 +343,7 @@ func TestClusterExtensionInstallRegistry(t *testing.T) {
},
},
Namespace: ns.Name,
- ServiceAccount: &ocv1.ServiceAccountReference{
+ ServiceAccount: ocv1.ServiceAccountReference{
Name: sa.Name,
},
}
@@ -403,7 +403,7 @@ func TestClusterExtensionInstallRegistryDynamic(t *testing.T) {
},
},
Namespace: ns.Name,
- ServiceAccount: &ocv1.ServiceAccountReference{
+ ServiceAccount: ocv1.ServiceAccountReference{
Name: sa.Name,
},
}
@@ -482,7 +482,7 @@ func TestClusterExtensionInstallRegistryMultipleBundles(t *testing.T) {
},
},
Namespace: ns.Name,
- ServiceAccount: &ocv1.ServiceAccountReference{
+ ServiceAccount: ocv1.ServiceAccountReference{
Name: sa.Name,
},
}
@@ -526,7 +526,7 @@ func TestClusterExtensionBlockInstallNonSuccessorVersion(t *testing.T) {
},
},
Namespace: ns.Name,
- ServiceAccount: &ocv1.ServiceAccountReference{
+ ServiceAccount: ocv1.ServiceAccountReference{
Name: sa.Name,
},
}
@@ -588,7 +588,7 @@ func TestClusterExtensionForceInstallNonSuccessorVersion(t *testing.T) {
},
},
Namespace: ns.Name,
- ServiceAccount: &ocv1.ServiceAccountReference{
+ ServiceAccount: ocv1.ServiceAccountReference{
Name: sa.Name,
},
}
@@ -637,7 +637,7 @@ func TestClusterExtensionInstallSuccessorVersion(t *testing.T) {
},
},
Namespace: ns.Name,
- ServiceAccount: &ocv1.ServiceAccountReference{
+ ServiceAccount: ocv1.ServiceAccountReference{
Name: sa.Name,
},
}
@@ -692,7 +692,7 @@ func TestClusterExtensionInstallReResolvesWhenCatalogIsPatched(t *testing.T) {
},
},
Namespace: ns.Name,
- ServiceAccount: &ocv1.ServiceAccountReference{
+ ServiceAccount: ocv1.ServiceAccountReference{
Name: sa.Name,
},
}
@@ -773,7 +773,7 @@ func TestClusterExtensionInstallReResolvesWhenNewCatalog(t *testing.T) {
},
},
Namespace: ns.Name,
- ServiceAccount: &ocv1.ServiceAccountReference{
+ ServiceAccount: ocv1.ServiceAccountReference{
Name: sa.Name,
},
}
@@ -834,7 +834,7 @@ func TestClusterExtensionInstallReResolvesWhenManagedContentChanged(t *testing.T
},
},
Namespace: ns.Name,
- ServiceAccount: &ocv1.ServiceAccountReference{
+ ServiceAccount: ocv1.ServiceAccountReference{
Name: sa.Name,
},
}
@@ -897,7 +897,7 @@ func TestClusterExtensionRecoversFromInitialInstallFailedWhenFailureFixed(t *tes
},
},
Namespace: ns.Name,
- ServiceAccount: &ocv1.ServiceAccountReference{
+ ServiceAccount: ocv1.ServiceAccountReference{
Name: sa.Name,
},
}
diff --git a/test/extension-developer-e2e/extension_developer_test.go b/test/extension-developer-e2e/extension_developer_test.go
index 8e9d2fd88..c493887b0 100644
--- a/test/extension-developer-e2e/extension_developer_test.go
+++ b/test/extension-developer-e2e/extension_developer_test.go
@@ -19,7 +19,6 @@ import (
"sigs.k8s.io/controller-runtime/pkg/client"
ocv1 "github.com/operator-framework/operator-controller/api/v1"
- "github.com/operator-framework/operator-controller/internal/operator-controller/authentication"
)
func TestExtensionDeveloper(t *testing.T) {
@@ -55,6 +54,14 @@ func TestExtensionDeveloper(t *testing.T) {
installNamespace := "default"
+ sa := &corev1.ServiceAccount{
+ ObjectMeta: metav1.ObjectMeta{
+ Name: fmt.Sprintf("serviceaccount-%s", rand.String(8)),
+ Namespace: installNamespace,
+ },
+ }
+ require.NoError(t, c.Create(ctx, sa))
+
clusterExtension := &ocv1.ClusterExtension{
ObjectMeta: metav1.ObjectMeta{
Name: "registryv1",
@@ -67,6 +74,9 @@ func TestExtensionDeveloper(t *testing.T) {
},
},
Namespace: installNamespace,
+ ServiceAccount: ocv1.ServiceAccountReference{
+ Name: sa.Name,
+ },
},
}
@@ -172,8 +182,9 @@ func TestExtensionDeveloper(t *testing.T) {
},
Subjects: []rbacv1.Subject{
{
- Kind: "User",
- Name: authentication.SyntheticUserName(*clusterExtension),
+ Kind: "ServiceAccount",
+ Name: sa.Name,
+ Namespace: sa.Namespace,
},
},
RoleRef: rbacv1.RoleRef{
From 35814f0c324457fc91f3316b73eacb30cdc59cff Mon Sep 17 00:00:00 2001
From: Per Goncalves da Silva <pegoncal@redhat.com>
Date: Thu, 27 Feb 2025 12:46:26 +0100
Subject: [PATCH 03/13] Add featuregate
Signed-off-by: Per Goncalves da Silva <pegoncal@redhat.com>
---
internal/operator-controller/features/features.go | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/internal/operator-controller/features/features.go b/internal/operator-controller/features/features.go
index e8faa07f0..2e9083735 100644
--- a/internal/operator-controller/features/features.go
+++ b/internal/operator-controller/features/features.go
@@ -13,6 +13,7 @@ const (
// Ex: SomeFeature featuregate.Feature = "SomeFeature"
PreflightPermissions featuregate.Feature = "PreflightPermissions"
SingleOwnNamespaceInstallSupport featuregate.Feature = "SingleOwnNamespaceInstallSupport"
+ SyntheticPermissions featuregate.Feature = "SyntheticPermissions"
)
var operatorControllerFeatureGates = map[featuregate.Feature]featuregate.FeatureSpec{
@@ -32,6 +33,14 @@ var operatorControllerFeatureGates = map[featuregate.Feature]featuregate.Feature
PreRelease: featuregate.Alpha,
LockToDefault: false,
},
+
+ // SyntheticPermissions enables support for a synthetic user permission
+ // model to manage operator permission boundaries
+ SyntheticPermissions: {
+ Default: false,
+ PreRelease: featuregate.Alpha,
+ LockToDefault: false,
+ },
}
var OperatorControllerFeatureGate featuregate.MutableFeatureGate = featuregate.NewFeatureGate()
From 4a48a993e30a19959b43d2bef4f4fcc998f02bb3 Mon Sep 17 00:00:00 2001
From: Per Goncalves da Silva <pegoncal@redhat.com>
Date: Thu, 27 Feb 2025 13:21:07 +0100
Subject: [PATCH 04/13] Add unit tests
Signed-off-by: Per Goncalves da Silva <pegoncal@redhat.com>
---
.../operator-controller/action/restconfig.go | 30 ++++--
.../action/restconfig_test.go | 92 +++++++++++++++++++
.../authentication/synthetic_test.go | 43 +++++++++
3 files changed, 157 insertions(+), 8 deletions(-)
create mode 100644 internal/operator-controller/action/restconfig_test.go
create mode 100644 internal/operator-controller/authentication/synthetic_test.go
diff --git a/internal/operator-controller/action/restconfig.go b/internal/operator-controller/action/restconfig.go
index cc6816078..e38049aa2 100644
--- a/internal/operator-controller/action/restconfig.go
+++ b/internal/operator-controller/action/restconfig.go
@@ -11,22 +11,36 @@ import (
ocv1 "github.com/operator-framework/operator-controller/api/v1"
"github.com/operator-framework/operator-controller/internal/operator-controller/authentication"
+ "github.com/operator-framework/operator-controller/internal/operator-controller/features"
)
-func ClusterExtensionUserRestConfigMapper(tokenGetter *authentication.TokenGetter) func(ctx context.Context, o client.Object, c *rest.Config) (*rest.Config, error) {
- saRestConfigMapper := serviceAccountRestConfigMapper(tokenGetter)
- synthRestConfigMapper := sythenticUserRestConfigMapper()
+const syntheticServiceAccountName = "olmv1:synthetic"
+
+type clusterExtensionRestConfigMapper struct {
+ saRestConfigMapper func(ctx context.Context, o client.Object, c *rest.Config) (*rest.Config, error)
+ synthUserRestConfigMapper func(ctx context.Context, o client.Object, c *rest.Config) (*rest.Config, error)
+}
+func (m *clusterExtensionRestConfigMapper) mapper() func(ctx context.Context, o client.Object, c *rest.Config) (*rest.Config, error) {
+ synthAuthFeatureEnabled := features.OperatorControllerFeatureGate.Enabled(features.SyntheticPermissions)
return func(ctx context.Context, o client.Object, c *rest.Config) (*rest.Config, error) {
cExt := o.(*ocv1.ClusterExtension)
- if cExt.Spec.ServiceAccount != nil { //nolint:staticcheck
- return saRestConfigMapper(ctx, o, c)
+ if synthAuthFeatureEnabled && cExt.Spec.ServiceAccount.Name == syntheticServiceAccountName {
+ return m.synthUserRestConfigMapper(ctx, o, c)
}
- return synthRestConfigMapper(ctx, o, c)
+ return m.saRestConfigMapper(ctx, o, c)
+ }
+}
+
+func ClusterExtensionUserRestConfigMapper(tokenGetter *authentication.TokenGetter) func(ctx context.Context, o client.Object, c *rest.Config) (*rest.Config, error) {
+ m := &clusterExtensionRestConfigMapper{
+ saRestConfigMapper: serviceAccountRestConfigMapper(tokenGetter),
+ synthUserRestConfigMapper: syntheticUserRestConfigMapper(),
}
+ return m.mapper()
}
-func sythenticUserRestConfigMapper() func(ctx context.Context, o client.Object, c *rest.Config) (*rest.Config, error) {
+func syntheticUserRestConfigMapper() 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)
cc := rest.CopyConfig(c)
@@ -41,7 +55,7 @@ func serviceAccountRestConfigMapper(tokenGetter *authentication.TokenGetter) fun
return func(ctx context.Context, o client.Object, c *rest.Config) (*rest.Config, error) {
cExt := o.(*ocv1.ClusterExtension)
saKey := types.NamespacedName{
- Name: cExt.Spec.ServiceAccount.Name, //nolint:staticcheck
+ Name: cExt.Spec.ServiceAccount.Name,
Namespace: cExt.Spec.Namespace,
}
saConfig := rest.AnonymousClientConfig(c)
diff --git a/internal/operator-controller/action/restconfig_test.go b/internal/operator-controller/action/restconfig_test.go
new file mode 100644
index 000000000..d98a2dd58
--- /dev/null
+++ b/internal/operator-controller/action/restconfig_test.go
@@ -0,0 +1,92 @@
+package action
+
+import (
+ "context"
+ "testing"
+
+ "github.com/stretchr/testify/require"
+ "k8s.io/client-go/rest"
+ featuregatetesting "k8s.io/component-base/featuregate/testing"
+ "sigs.k8s.io/controller-runtime/pkg/client"
+
+ ocv1 "github.com/operator-framework/operator-controller/api/v1"
+ "github.com/operator-framework/operator-controller/internal/operator-controller/features"
+)
+
+const (
+ saAccountWrapper = "service account wrapper"
+ synthUserWrapper = "synthetic user wrapper"
+)
+
+func fakeRestConfigWrapper() clusterExtensionRestConfigMapper {
+ // The rest config's host field is artificially used to differentiate between the wrappers
+ return clusterExtensionRestConfigMapper{
+ saRestConfigMapper: func(ctx context.Context, o client.Object, c *rest.Config) (*rest.Config, error) {
+ return &rest.Config{
+ Host: saAccountWrapper,
+ }, nil
+ },
+ synthUserRestConfigMapper: func(ctx context.Context, o client.Object, c *rest.Config) (*rest.Config, error) {
+ return &rest.Config{
+ Host: synthUserWrapper,
+ }, nil
+ },
+ }
+}
+
+func TestMapper_SyntheticPermissionsEnabled(t *testing.T) {
+ featuregatetesting.SetFeatureGateDuringTest(t, features.OperatorControllerFeatureGate, features.SyntheticPermissions, true)
+
+ for _, tc := range []struct {
+ description string
+ serviceAccountName string
+ expectedMapper string
+ fgEnabled bool
+ }{
+ {
+ description: "user service account wrapper if extension service account is _not_ called olmv1:synthetic",
+ serviceAccountName: "_not_:olmv1:synthetic",
+ expectedMapper: saAccountWrapper,
+ fgEnabled: true,
+ }, {
+ description: "user synthetic user wrapper is extension service account is called olmv1:synthetic",
+ serviceAccountName: "olmv1:synthetic",
+ expectedMapper: synthUserWrapper,
+ fgEnabled: true,
+ },
+ } {
+ t.Run(tc.description, func(t *testing.T) {
+ m := fakeRestConfigWrapper()
+ mapper := m.mapper()
+ ext := &ocv1.ClusterExtension{
+ Spec: ocv1.ClusterExtensionSpec{
+ ServiceAccount: ocv1.ServiceAccountReference{
+ Name: tc.serviceAccountName,
+ },
+ },
+ }
+ cfg, err := mapper(context.Background(), ext, &rest.Config{})
+ require.NoError(t, err)
+
+ // The rest config's host field is artificially used to differentiate between the wrappers
+ require.Equal(t, tc.expectedMapper, cfg.Host)
+ })
+ }
+}
+
+func TestMapper_SyntheticPermissionsDisabled(t *testing.T) {
+ m := fakeRestConfigWrapper()
+ mapper := m.mapper()
+ ext := &ocv1.ClusterExtension{
+ Spec: ocv1.ClusterExtensionSpec{
+ ServiceAccount: ocv1.ServiceAccountReference{
+ Name: "olmv1:synthetic",
+ },
+ },
+ }
+ cfg, err := mapper(context.Background(), ext, &rest.Config{})
+ require.NoError(t, err)
+
+ // The rest config's host field is artificially used to differentiate between the wrappers
+ require.Equal(t, saAccountWrapper, cfg.Host)
+}
diff --git a/internal/operator-controller/authentication/synthetic_test.go b/internal/operator-controller/authentication/synthetic_test.go
new file mode 100644
index 000000000..081140894
--- /dev/null
+++ b/internal/operator-controller/authentication/synthetic_test.go
@@ -0,0 +1,43 @@
+package authentication_test
+
+import (
+ "testing"
+
+ "github.com/stretchr/testify/require"
+ metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+
+ ocv1 "github.com/operator-framework/operator-controller/api/v1"
+ "github.com/operator-framework/operator-controller/internal/operator-controller/authentication"
+)
+
+func TestSyntheticUserName(t *testing.T) {
+ syntheticUserName := authentication.SyntheticUserName(ocv1.ClusterExtension{
+ ObjectMeta: metav1.ObjectMeta{
+ Name: "my-ext",
+ },
+ })
+ require.Equal(t, "olmv1:clusterextensions:my-ext:admin", syntheticUserName)
+}
+
+func TestSyntheticGroups(t *testing.T) {
+ syntheticGroups := authentication.SyntheticGroups(ocv1.ClusterExtension{})
+ require.Equal(t, []string{
+ "olmv1:clusterextensions:admin",
+ "system:authenticated",
+ }, syntheticGroups)
+}
+
+func TestSyntheticImpersonationConfig(t *testing.T) {
+ config := authentication.SyntheticImpersonationConfig(ocv1.ClusterExtension{
+ ObjectMeta: metav1.ObjectMeta{
+ Name: "my-ext",
+ },
+ })
+ require.Equal(t, "olmv1:clusterextensions:my-ext:admin", config.UserName)
+ require.Equal(t, []string{
+ "olmv1:clusterextensions:admin",
+ "system:authenticated",
+ }, config.Groups)
+ require.Empty(t, config.UID)
+ require.Empty(t, config.Extra)
+}
From e1bbb1f9ca38b365aac6a84aa1ce565ae0971e03 Mon Sep 17 00:00:00 2001
From: Per Goncalves da Silva <pegoncal@redhat.com>
Date: Thu, 27 Feb 2025 14:55:05 +0100
Subject: [PATCH 05/13] Update syntheric user format
Signed-off-by: Per Goncalves da Silva <pegoncal@redhat.com>
---
internal/operator-controller/action/restconfig.go | 2 +-
internal/operator-controller/action/restconfig_test.go | 10 +++++-----
.../operator-controller/authentication/synthetic.go | 4 ++--
.../authentication/synthetic_test.go | 8 ++++----
4 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/internal/operator-controller/action/restconfig.go b/internal/operator-controller/action/restconfig.go
index e38049aa2..b83696621 100644
--- a/internal/operator-controller/action/restconfig.go
+++ b/internal/operator-controller/action/restconfig.go
@@ -14,7 +14,7 @@ import (
"github.com/operator-framework/operator-controller/internal/operator-controller/features"
)
-const syntheticServiceAccountName = "olmv1:synthetic"
+const syntheticServiceAccountName = "olm.synthetic-user"
type clusterExtensionRestConfigMapper struct {
saRestConfigMapper func(ctx context.Context, o client.Object, c *rest.Config) (*rest.Config, error)
diff --git a/internal/operator-controller/action/restconfig_test.go b/internal/operator-controller/action/restconfig_test.go
index d98a2dd58..94e9bf807 100644
--- a/internal/operator-controller/action/restconfig_test.go
+++ b/internal/operator-controller/action/restconfig_test.go
@@ -44,13 +44,13 @@ func TestMapper_SyntheticPermissionsEnabled(t *testing.T) {
fgEnabled bool
}{
{
- description: "user service account wrapper if extension service account is _not_ called olmv1:synthetic",
- serviceAccountName: "_not_:olmv1:synthetic",
+ description: "user service account wrapper if extension service account is _not_ called olm.synthetic-user",
+ serviceAccountName: "not.olm.synthetic-user",
expectedMapper: saAccountWrapper,
fgEnabled: true,
}, {
- description: "user synthetic user wrapper is extension service account is called olmv1:synthetic",
- serviceAccountName: "olmv1:synthetic",
+ description: "user synthetic user wrapper is extension service account is called olm.synthetic-user",
+ serviceAccountName: "olm.synthetic-user",
expectedMapper: synthUserWrapper,
fgEnabled: true,
},
@@ -80,7 +80,7 @@ func TestMapper_SyntheticPermissionsDisabled(t *testing.T) {
ext := &ocv1.ClusterExtension{
Spec: ocv1.ClusterExtensionSpec{
ServiceAccount: ocv1.ServiceAccountReference{
- Name: "olmv1:synthetic",
+ Name: "olm.synthetic-user",
},
},
}
diff --git a/internal/operator-controller/authentication/synthetic.go b/internal/operator-controller/authentication/synthetic.go
index 0dbb754e9..9a01a0761 100644
--- a/internal/operator-controller/authentication/synthetic.go
+++ b/internal/operator-controller/authentication/synthetic.go
@@ -9,12 +9,12 @@ import (
)
func SyntheticUserName(ext ocv1.ClusterExtension) string {
- return fmt.Sprintf("olmv1:clusterextensions:%s:admin", ext.Name)
+ return fmt.Sprintf("olm:clusterextensions:%s", ext.Name)
}
func SyntheticGroups(_ ocv1.ClusterExtension) []string {
return []string{
- "olmv1:clusterextensions:admin",
+ "olm:clusterextensions",
"system:authenticated",
}
}
diff --git a/internal/operator-controller/authentication/synthetic_test.go b/internal/operator-controller/authentication/synthetic_test.go
index 081140894..d50686a21 100644
--- a/internal/operator-controller/authentication/synthetic_test.go
+++ b/internal/operator-controller/authentication/synthetic_test.go
@@ -16,13 +16,13 @@ func TestSyntheticUserName(t *testing.T) {
Name: "my-ext",
},
})
- require.Equal(t, "olmv1:clusterextensions:my-ext:admin", syntheticUserName)
+ require.Equal(t, "olm:clusterextensions:my-ext", syntheticUserName)
}
func TestSyntheticGroups(t *testing.T) {
syntheticGroups := authentication.SyntheticGroups(ocv1.ClusterExtension{})
require.Equal(t, []string{
- "olmv1:clusterextensions:admin",
+ "olm:clusterextensions",
"system:authenticated",
}, syntheticGroups)
}
@@ -33,9 +33,9 @@ func TestSyntheticImpersonationConfig(t *testing.T) {
Name: "my-ext",
},
})
- require.Equal(t, "olmv1:clusterextensions:my-ext:admin", config.UserName)
+ require.Equal(t, "olm:clusterextensions:my-ext", config.UserName)
require.Equal(t, []string{
- "olmv1:clusterextensions:admin",
+ "olm:clusterextensions",
"system:authenticated",
}, config.Groups)
require.Empty(t, config.UID)
From 743611abee14492bdd106f81785ba4a3036055f2 Mon Sep 17 00:00:00 2001
From: Per Goncalves da Silva <pegoncal@redhat.com>
Date: Fri, 28 Feb 2025 08:35:24 +0100
Subject: [PATCH 06/13] Add featuregate kustomize overlay
Signed-off-by: Per Goncalves da Silva <pegoncal@redhat.com>
---
.../kustomization.yaml | 19 +++++++++++++++++++
.../patches/enable-featuregate.yaml | 4 ++++
.../patches/impersonate-perms.yaml | 11 +++++++++++
3 files changed, 34 insertions(+)
create mode 100644 config/overlays/featuregate/synthetic-user-permissions/kustomization.yaml
create mode 100644 config/overlays/featuregate/synthetic-user-permissions/patches/enable-featuregate.yaml
create mode 100644 config/overlays/featuregate/synthetic-user-permissions/patches/impersonate-perms.yaml
diff --git a/config/overlays/featuregate/synthetic-user-permissions/kustomization.yaml b/config/overlays/featuregate/synthetic-user-permissions/kustomization.yaml
new file mode 100644
index 000000000..01e3a6d0e
--- /dev/null
+++ b/config/overlays/featuregate/synthetic-user-permissions/kustomization.yaml
@@ -0,0 +1,19 @@
+# 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
+ - target:
+ kind: ClusterRole
+ name: operator-controller-manager-role
+ path: patches/impersonate-perms.yaml
diff --git a/config/overlays/featuregate/synthetic-user-permissions/patches/enable-featuregate.yaml b/config/overlays/featuregate/synthetic-user-permissions/patches/enable-featuregate.yaml
new file mode 100644
index 000000000..fb6c84fa4
--- /dev/null
+++ b/config/overlays/featuregate/synthetic-user-permissions/patches/enable-featuregate.yaml
@@ -0,0 +1,4 @@
+# enable synthetic-user feature gate
+- op: add
+ path: /spec/template/spec/containers/0/args/-
+ value: "--feature-gates=SyntheticPermissions=true"
diff --git a/config/overlays/featuregate/synthetic-user-permissions/patches/impersonate-perms.yaml b/config/overlays/featuregate/synthetic-user-permissions/patches/impersonate-perms.yaml
new file mode 100644
index 000000000..f3854ea2a
--- /dev/null
+++ b/config/overlays/featuregate/synthetic-user-permissions/patches/impersonate-perms.yaml
@@ -0,0 +1,11 @@
+# enable synthetic-user feature gate
+- op: add
+ path: /rules/-
+ value:
+ apiGroups:
+ - ""
+ resources:
+ - groups
+ - users
+ verbs:
+ - impersonate
From b9d10e397672f8248e591da1ee5709df00ebb928 Mon Sep 17 00:00:00 2001
From: Per Goncalves da Silva <pegoncal@redhat.com>
Date: Fri, 28 Feb 2025 10:49:37 +0100
Subject: [PATCH 07/13] Add demo resources
Signed-off-by: Per Goncalves da Silva <pegoncal@redhat.com>
---
.../resources/synthetic-user-perms-demo.yaml | 127 ++++++++++++++++++
1 file changed, 127 insertions(+)
create mode 100644 hack/demo/resources/synthetic-user-perms-demo.yaml
diff --git a/hack/demo/resources/synthetic-user-perms-demo.yaml b/hack/demo/resources/synthetic-user-perms-demo.yaml
new file mode 100644
index 000000000..530917b01
--- /dev/null
+++ b/hack/demo/resources/synthetic-user-perms-demo.yaml
@@ -0,0 +1,127 @@
+---
+apiVersion: v1
+kind: Namespace
+metadata:
+ name: argocd-system
+---
+apiVersion: rbac.authorization.k8s.io/v1
+kind: ClusterRole
+metadata:
+ name: clusterextension-installer
+rules:
+ - apiGroups: [ olm.operatorframework.io ]
+ resources: [ clusterextensions/finalizers ]
+ verbs: [ update ]
+ - apiGroups: [ apiextensions.k8s.io ]
+ resources: [ customresourcedefinitions ]
+ verbs: [ create, list, watch, get, update, patch, delete ]
+ - apiGroups: [ rbac.authorization.k8s.io ]
+ resources: [ clusterroles, roles, clusterrolebindings, rolebindings ]
+ verbs: [ create, list, watch, get, update, patch, delete ]
+ - apiGroups: [""]
+ resources: [configmaps, endpoints, events, pods, pod/logs, serviceaccounts, services, services/finalizers, namespaces, persistentvolumeclaims]
+ verbs: ['*']
+ - apiGroups: [apps]
+ resources: [ '*' ]
+ verbs: ['*']
+ - apiGroups: [ batch ]
+ resources: [ '*' ]
+ verbs: [ '*' ]
+ - apiGroups: [ networking.k8s.io ]
+ resources: [ '*' ]
+ verbs: [ '*' ]
+ - apiGroups: [authentication.k8s.io]
+ resources: [tokenreviews, subjectaccessreviews]
+ verbs: [create]
+ - apiGroups: [autoscaling]
+ resources: [horizontalpodautoscalers]
+ verbs: ['*']
+ - apiGroups: [ apps.openshift.io ]
+ resources: [ '*' ]
+ verbs: [ '*' ]
+ - apiGroups: [config.openshift.io]
+ resources: [clusterversions]
+ verbs: [get, list, watch]
+ - apiGroups: [monitoring.coreos.com]
+ resources: ['*']
+ verbs: ['*']
+ - apiGroups: [oauth.openshift.io]
+ resources: [oauthclients]
+ verbs: [create, delete, get, list, patch, update, watch]
+ - apiGroups: [rbac.authorization.k8s.io]
+ resources: ['*']
+ verbs: ['*']
+ - apiGroups: [route.openshift.io]
+ resources: ['*']
+ verbs: ['*']
+ - apiGroups: [template.openshift.io]
+ resources: ['*']
+ verbs: ['*']
+---
+apiVersion: rbac.authorization.k8s.io/v1
+kind: ClusterRole
+metadata:
+ name: clusterextension-privileged
+rules:
+ - apiGroups: [""]
+ resources: [secrets]
+ verbs: ['*']
+---
+apiVersion: rbac.authorization.k8s.io/v1
+kind: ClusterRole
+metadata:
+ name: clusterextension-installer-crds
+rules:
+ - apiGroups: [argoproj.io]
+ resources: ['*']
+ verbs: ['*']
+---
+apiVersion: rbac.authorization.k8s.io/v1
+kind: ClusterRoleBinding
+metadata:
+ name: clusterextension-installer-binding
+roleRef:
+ apiGroup: rbac.authorization.k8s.io
+ kind: ClusterRole
+ name: clusterextension-installer
+subjects:
+ - kind: User
+ name: "olm:clusterextensions:argocd-operator"
+---
+apiVersion: rbac.authorization.k8s.io/v1
+kind: ClusterRoleBinding
+metadata:
+ name: clusterextension-privileged-binding
+roleRef:
+ apiGroup: rbac.authorization.k8s.io
+ kind: ClusterRole
+ name: clusterextension-privileged
+subjects:
+ - kind: User
+ name: "olm:clusterextensions:argocd-operator"
+---
+apiVersion: rbac.authorization.k8s.io/v1
+kind: ClusterRoleBinding
+metadata:
+ name: clusterextension-installer-crds-binding
+roleRef:
+ apiGroup: rbac.authorization.k8s.io
+ kind: ClusterRole
+ name: clusterextension-installer-crds
+subjects:
+ - kind: User
+ name: "olm:clusterextensions:argocd-operator"
+---
+apiVersion: olm.operatorframework.io/v1
+kind: ClusterExtension
+metadata:
+ name: argocd-operator
+spec:
+ namespace: argocd-system
+ serviceAccount:
+ name: "olm.synthetic-user"
+ source:
+ sourceType: Catalog
+ catalog:
+ packageName: argocd-operator
+ version: 0.6.0
From b46041dfad8fd90b707fe4e964334834413b5f07 Mon Sep 17 00:00:00 2001
From: Per Goncalves da Silva <pegoncal@redhat.com>
Date: Tue, 4 Mar 2025 09:53:00 +0100
Subject: [PATCH 08/13] Refactor synthetic auth exported functions
Signed-off-by: Per Goncalves da Silva <pegoncal@redhat.com>
---
.../authentication/synthetic.go | 8 ++++----
.../authentication/synthetic_test.go | 17 -----------------
2 files changed, 4 insertions(+), 21 deletions(-)
diff --git a/internal/operator-controller/authentication/synthetic.go b/internal/operator-controller/authentication/synthetic.go
index 9a01a0761..a07a88d91 100644
--- a/internal/operator-controller/authentication/synthetic.go
+++ b/internal/operator-controller/authentication/synthetic.go
@@ -8,11 +8,11 @@ import (
ocv1 "github.com/operator-framework/operator-controller/api/v1"
)
-func SyntheticUserName(ext ocv1.ClusterExtension) string {
+func syntheticUserName(ext ocv1.ClusterExtension) string {
return fmt.Sprintf("olm:clusterextensions:%s", ext.Name)
}
-func SyntheticGroups(_ ocv1.ClusterExtension) []string {
+func syntheticGroups(_ ocv1.ClusterExtension) []string {
return []string{
"olm:clusterextensions",
"system:authenticated",
@@ -21,7 +21,7 @@ func SyntheticGroups(_ ocv1.ClusterExtension) []string {
func SyntheticImpersonationConfig(ext ocv1.ClusterExtension) transport.ImpersonationConfig {
return transport.ImpersonationConfig{
- UserName: SyntheticUserName(ext),
- Groups: SyntheticGroups(ext),
+ UserName: syntheticUserName(ext),
+ Groups: syntheticGroups(ext),
}
}
diff --git a/internal/operator-controller/authentication/synthetic_test.go b/internal/operator-controller/authentication/synthetic_test.go
index d50686a21..f27025444 100644
--- a/internal/operator-controller/authentication/synthetic_test.go
+++ b/internal/operator-controller/authentication/synthetic_test.go
@@ -10,23 +10,6 @@ import (
"github.com/operator-framework/operator-controller/internal/operator-controller/authentication"
)
-func TestSyntheticUserName(t *testing.T) {
- syntheticUserName := authentication.SyntheticUserName(ocv1.ClusterExtension{
- ObjectMeta: metav1.ObjectMeta{
- Name: "my-ext",
- },
- })
- require.Equal(t, "olm:clusterextensions:my-ext", syntheticUserName)
-}
-
-func TestSyntheticGroups(t *testing.T) {
- syntheticGroups := authentication.SyntheticGroups(ocv1.ClusterExtension{})
- require.Equal(t, []string{
- "olm:clusterextensions",
- "system:authenticated",
- }, syntheticGroups)
-}
-
func TestSyntheticImpersonationConfig(t *testing.T) {
config := authentication.SyntheticImpersonationConfig(ocv1.ClusterExtension{
ObjectMeta: metav1.ObjectMeta{
From 547b72e9d584fcf019ac94b96da7888f30111a4b Mon Sep 17 00:00:00 2001
From: Per Goncalves da Silva <pegoncal@redhat.com>
Date: Tue, 4 Mar 2025 10:37:03 +0100
Subject: [PATCH 09/13] Update demo
Signed-off-by: Per Goncalves da Silva <pegoncal@redhat.com>
---
.../argocd-clusterextension.yaml | 13 ++++++++
.../cegroup-admin-binding.yaml | 11 +++++++
.../synthetic-user-perms-demo.yaml | 19 ------------
.../demo/synthetic-user-cluster-admin-demo.sh | 30 +++++++++++++++++++
4 files changed, 54 insertions(+), 19 deletions(-)
create mode 100644 hack/demo/resources/synthetic-user-perms/argocd-clusterextension.yaml
create mode 100644 hack/demo/resources/synthetic-user-perms/cegroup-admin-binding.yaml
rename hack/demo/resources/{ => synthetic-user-perms}/synthetic-user-perms-demo.yaml (89%)
create mode 100755 hack/demo/synthetic-user-cluster-admin-demo.sh
diff --git a/hack/demo/resources/synthetic-user-perms/argocd-clusterextension.yaml b/hack/demo/resources/synthetic-user-perms/argocd-clusterextension.yaml
new file mode 100644
index 000000000..a8dc8309f
--- /dev/null
+++ b/hack/demo/resources/synthetic-user-perms/argocd-clusterextension.yaml
@@ -0,0 +1,13 @@
+apiVersion: olm.operatorframework.io/v1
+kind: ClusterExtension
+metadata:
+ name: argocd-operator
+spec:
+ namespace: argocd-system
+ serviceAccount:
+ name: "olm.synthetic-user"
+ source:
+ sourceType: Catalog
+ catalog:
+ packageName: argocd-operator
+ version: 0.6.0
\ No newline at end of file
diff --git a/hack/demo/resources/synthetic-user-perms/cegroup-admin-binding.yaml b/hack/demo/resources/synthetic-user-perms/cegroup-admin-binding.yaml
new file mode 100644
index 000000000..069d0ea7a
--- /dev/null
+++ b/hack/demo/resources/synthetic-user-perms/cegroup-admin-binding.yaml
@@ -0,0 +1,11 @@
+apiVersion: rbac.authorization.k8s.io/v1
+kind: ClusterRoleBinding
+metadata:
+ name: clusterextensions-group-admin-binding
+roleRef:
+ apiGroup: rbac.authorization.k8s.io
+ kind: ClusterRole
+ name: cluster-admin
+subjects:
+ - kind: Group
+ name: "olm:clusterextensions"
\ No newline at end of file
diff --git a/hack/demo/resources/synthetic-user-perms-demo.yaml b/hack/demo/resources/synthetic-user-perms/synthetic-user-perms-demo.yaml
similarity index 89%
rename from hack/demo/resources/synthetic-user-perms-demo.yaml
rename to hack/demo/resources/synthetic-user-perms/synthetic-user-perms-demo.yaml
index 530917b01..dc7113364 100644
--- a/hack/demo/resources/synthetic-user-perms-demo.yaml
+++ b/hack/demo/resources/synthetic-user-perms/synthetic-user-perms-demo.yaml
@@ -1,9 +1,4 @@
---
-apiVersion: v1
-kind: Namespace
-metadata:
- name: argocd-system
----
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRole
metadata:
@@ -111,17 +106,3 @@ roleRef:
subjects:
- kind: User
name: "olm:clusterextensions:argocd-operator"
----
-apiVersion: olm.operatorframework.io/v1
-kind: ClusterExtension
-metadata:
- name: argocd-operator
-spec:
- namespace: argocd-system
- serviceAccount:
- name: "olm.synthetic-user"
- source:
- sourceType: Catalog
- catalog:
- packageName: argocd-operator
- version: 0.6.0
diff --git a/hack/demo/synthetic-user-cluster-admin-demo.sh b/hack/demo/synthetic-user-cluster-admin-demo.sh
new file mode 100755
index 000000000..4790e46e7
--- /dev/null
+++ b/hack/demo/synthetic-user-cluster-admin-demo.sh
@@ -0,0 +1,30 @@
+#!/usr/bin/env bash
+
+#
+# Welcome to the SingleNamespace install mode demo
+#
+trap "trap - SIGTERM && kill -- -$$" SIGINT SIGTERM EXIT
+
+# enable 'SyntheticPermissions' feature
+kubectl kustomize config/overlays/featuregate/synthetic-user-permissions | kubectl apply -f -
+
+# wait for operator-controller to become available
+kubectl rollout status -n olmv1-system deployment/operator-controller-controller-manager
+
+# create install namespace
+kubectl create ns argocd-system
+
+# give cluster extension group cluster admin privileges - all cluster extensions installer users will be cluster admin
+bat --style=plain ${DEMO_RESOURCE_DIR}/synthetic-user-perms/cegroup-admin-binding.yaml
+
+# apply cluster role binding
+kubectl apply -f ${DEMO_RESOURCE_DIR}/synthetic-user-perms/cegroup-admin-binding.yaml
+
+# install cluster extension - for now .spec.serviceAccount = "olm.synthetic-user"
+bat --style=plain ${DEMO_RESOURCE_DIR}/synthetic-user-perms/argocd-clusterextension.yaml
+
+# apply cluster extension
+kubectl apply -f ${DEMO_RESOURCE_DIR}/synthetic-user-perms/argocd-clusterextension.yaml
+
+# wait for cluster extension installation to succeed
+kubectl wait --for=condition=Installed clusterextension/argocd-operator --timeout="60s"
From 97f2320ef55c247c5ee478ae98b8490d1b632048 Mon Sep 17 00:00:00 2001
From: Per Goncalves da Silva <pegoncal@redhat.com>
Date: Tue, 4 Mar 2025 11:05:53 +0100
Subject: [PATCH 10/13] Add some docs
Signed-off-by: Per Goncalves da Silva <pegoncal@redhat.com>
---
docs/draft/howto/use-synthetic-permissions.md | 133 ++++++++++++++++++
1 file changed, 133 insertions(+)
create mode 100644 docs/draft/howto/use-synthetic-permissions.md
diff --git a/docs/draft/howto/use-synthetic-permissions.md b/docs/draft/howto/use-synthetic-permissions.md
new file mode 100644
index 000000000..15f9c2c20
--- /dev/null
+++ b/docs/draft/howto/use-synthetic-permissions.md
@@ -0,0 +1,133 @@
+## Synthetic User Permissions
+
+!!! note
+This feature is still in *alpha* the `SyntheticPermissions` feature-gate must be enabled to make use of it.
+See the instructions below on how to enable it.
+
+Synthetic user permissions enables fine-grained configuration of ClusterExtension management client RBAC permissions.
+User can not only configure RBAC permissions governing the management across all ClusterExtensions, but also on a
+case-by-case basis.
+
+### Update OLM to enable Feature
+
+```terminal title=Enable SyntheticPermissions feature
+kubectl kustomize config/overlays/featuregate/synthetic-user-permissions | kubectl apply -f -
+```
+
+```terminal title=Wait for rollout to complete
+kubectl rollout status -n olmv1-system deployment/operator-controller-controller-manager
+```
+
+### How does it work?
+
+When managing a ClusterExtension, OLM will assume the identity of user "olm:clusterextensions:<clusterextension-name>"
+and group "olm:clusterextensions" limiting Kubernetes API access scope to those defined for this user and group. These
+users and group do not exist beyond being defined in Cluster/RoleBinding(s) and can only be impersonated by clients with
+ `impersonate` verb permissions on the `users` and `groups` resources.
+
+### Demo
+
+[](https://asciinema.org/a/Jbtt8nkV8Dm7vriHxq7sxiVvi)
+
+#### Examples:
+
+##### ClusterExtension management as cluster-admin
+
+To enable ClusterExtensions management as cluster-admin, bind the `cluster-admin` cluster role to the `olm:clusterextensions`
+group:
+
+```
+apiVersion: rbac.authorization.k8s.io/v1
+kind: ClusterRoleBinding
+metadata:
+ name: clusterextensions-group-admin-binding
+roleRef:
+ apiGroup: rbac.authorization.k8s.io
+ kind: ClusterRole
+ name: cluster-admin
+subjects:
+- kind: Group
+ name: "olm:clusterextensions"
+```
+
+##### Scoped olm:clusterextension group + Added perms on specific extensions
+
+Give ClusterExtension management group broad permissions to manage ClusterExtensions denying potentially dangerous
+permissions such as being able to read cluster wide secrets:
+
+```
+apiVersion: rbac.authorization.k8s.io/v1
+kind: ClusterRole
+metadata:
+ name: clusterextension-installer
+rules:
+ - apiGroups: [ olm.operatorframework.io ]
+ resources: [ clusterextensions/finalizers ]
+ verbs: [ update ]
+ - apiGroups: [ apiextensions.k8s.io ]
+ resources: [ customresourcedefinitions ]
+ verbs: [ create, list, watch, get, update, patch, delete ]
+ - apiGroups: [ rbac.authorization.k8s.io ]
+ resources: [ clusterroles, roles, clusterrolebindings, rolebindings ]
+ verbs: [ create, list, watch, get, update, patch, delete ]
+ - apiGroups: [""]
+ resources: [configmaps, endpoints, events, pods, pod/logs, serviceaccounts, services, services/finalizers, namespaces, persistentvolumeclaims]
+ verbs: ['*']
+ - apiGroups: [apps]
+ resources: [ '*' ]
+ verbs: ['*']
+ - apiGroups: [ batch ]
+ resources: [ '*' ]
+ verbs: [ '*' ]
+ - apiGroups: [ networking.k8s.io ]
+ resources: [ '*' ]
+ verbs: [ '*' ]
+ - apiGroups: [authentication.k8s.io]
+ resources: [tokenreviews, subjectaccessreviews]
+ verbs: [create]
+```
+
+```
+apiVersion: rbac.authorization.k8s.io/v1
+kind: ClusterRoleBinding
+metadata:
+ name: clusterextension-installer-binding
+roleRef:
+ apiGroup: rbac.authorization.k8s.io
+ kind: ClusterRole
+ name: clusterextension-installer
+subjects:
+- kind: Group
+ name: "olm:clusterextensions"
+```
+
+Give a specific ClusterExtension secrets access, maybe even on specific namespaces:
+
+```
+apiVersion: rbac.authorization.k8s.io/v1
+kind: ClusterRole
+metadata:
+ name: clusterextension-privileged
+rules:
+- apiGroups: [""]
+ resources: [secrets]
+ verbs: ['*']
+```
+
+```
+apiVersion: rbac.authorization.k8s.io/v1
+kind: RoleBinding
+metadata:
+ name: clusterextension-privileged-binding
+ namespace: <some namespace>
+roleRef:
+ apiGroup: rbac.authorization.k8s.io
+ kind: ClusterRole
+ name: clusterextension-privileged
+subjects:
+- kind: User
+ name: "olm:clusterextensions:argocd-operator"
+```
+
+Note: In this example the ClusterExtension user (or group) will still need to be updated to be able to manage
+the CRs coming from the argocd operator. Some look ahead and RBAC permission wrangling will still be required.
From dc4067c8fb727d44afaa9956452db559e1249740 Mon Sep 17 00:00:00 2001
From: Per Goncalves da Silva <pegoncal@redhat.com>
Date: Tue, 4 Mar 2025 11:06:38 +0100
Subject: [PATCH 11/13] Clean up demo resources
Signed-off-by: Per Goncalves da Silva <pegoncal@redhat.com>
---
.../synthetic-user-perms-demo.yaml | 108 ------------------
1 file changed, 108 deletions(-)
delete mode 100644 hack/demo/resources/synthetic-user-perms/synthetic-user-perms-demo.yaml
diff --git a/hack/demo/resources/synthetic-user-perms/synthetic-user-perms-demo.yaml b/hack/demo/resources/synthetic-user-perms/synthetic-user-perms-demo.yaml
deleted file mode 100644
index dc7113364..000000000
--- a/hack/demo/resources/synthetic-user-perms/synthetic-user-perms-demo.yaml
+++ /dev/null
@@ -1,108 +0,0 @@
----
-apiVersion: rbac.authorization.k8s.io/v1
-kind: ClusterRole
-metadata:
- name: clusterextension-installer
-rules:
- - apiGroups: [ olm.operatorframework.io ]
- resources: [ clusterextensions/finalizers ]
- verbs: [ update ]
- - apiGroups: [ apiextensions.k8s.io ]
- resources: [ customresourcedefinitions ]
- verbs: [ create, list, watch, get, update, patch, delete ]
- - apiGroups: [ rbac.authorization.k8s.io ]
- resources: [ clusterroles, roles, clusterrolebindings, rolebindings ]
- verbs: [ create, list, watch, get, update, patch, delete ]
- - apiGroups: [""]
- resources: [configmaps, endpoints, events, pods, pod/logs, serviceaccounts, services, services/finalizers, namespaces, persistentvolumeclaims]
- verbs: ['*']
- - apiGroups: [apps]
- resources: [ '*' ]
- verbs: ['*']
- - apiGroups: [ batch ]
- resources: [ '*' ]
- verbs: [ '*' ]
- - apiGroups: [ networking.k8s.io ]
- resources: [ '*' ]
- verbs: [ '*' ]
- - apiGroups: [authentication.k8s.io]
- resources: [tokenreviews, subjectaccessreviews]
- verbs: [create]
- - apiGroups: [autoscaling]
- resources: [horizontalpodautoscalers]
- verbs: ['*']
- - apiGroups: [ apps.openshift.io ]
- resources: [ '*' ]
- verbs: [ '*' ]
- - apiGroups: [config.openshift.io]
- resources: [clusterversions]
- verbs: [get, list, watch]
- - apiGroups: [monitoring.coreos.com]
- resources: ['*']
- verbs: ['*']
- - apiGroups: [oauth.openshift.io]
- resources: [oauthclients]
- verbs: [create, delete, get, list, patch, update, watch]
- - apiGroups: [rbac.authorization.k8s.io]
- resources: ['*']
- verbs: ['*']
- - apiGroups: [route.openshift.io]
- resources: ['*']
- verbs: ['*']
- - apiGroups: [template.openshift.io]
- resources: ['*']
- verbs: ['*']
----
-apiVersion: rbac.authorization.k8s.io/v1
-kind: ClusterRole
-metadata:
- name: clusterextension-privileged
-rules:
- - apiGroups: [""]
- resources: [secrets]
- verbs: ['*']
----
-apiVersion: rbac.authorization.k8s.io/v1
-kind: ClusterRole
-metadata:
- name: clusterextension-installer-crds
-rules:
- - apiGroups: [argoproj.io]
- resources: ['*']
- verbs: ['*']
----
-apiVersion: rbac.authorization.k8s.io/v1
-kind: ClusterRoleBinding
-metadata:
- name: clusterextension-installer-binding
-roleRef:
- apiGroup: rbac.authorization.k8s.io
- kind: ClusterRole
- name: clusterextension-installer
-subjects:
- - kind: User
- name: "olm:clusterextensions:argocd-operator"
----
-apiVersion: rbac.authorization.k8s.io/v1
-kind: ClusterRoleBinding
-metadata:
- name: clusterextension-privileged-binding
-roleRef:
- apiGroup: rbac.authorization.k8s.io
- kind: ClusterRole
- name: clusterextension-privileged
-subjects:
- - kind: User
- name: "olm:clusterextensions:argocd-operator"
----
-apiVersion: rbac.authorization.k8s.io/v1
-kind: ClusterRoleBinding
-metadata:
- name: clusterextension-installer-crds-binding
-roleRef:
- apiGroup: rbac.authorization.k8s.io
- kind: ClusterRole
- name: clusterextension-installer-crds
-subjects:
- - kind: User
- name: "olm:clusterextensions:argocd-operator"
From 4bc2f8e58959a96f0def0c43069f0bf65887c3c9 Mon Sep 17 00:00:00 2001
From: Per Goncalves da Silva <pegoncal@redhat.com>
Date: Wed, 5 Mar 2025 14:53:22 +0100
Subject: [PATCH 12/13] Address reviewer comments
Signed-off-by: Per Goncalves da Silva <pegoncal@redhat.com>
---
.../synthetic-user-perms/argocd-clusterextension.yaml | 2 +-
.../resources/synthetic-user-perms/cegroup-admin-binding.yaml | 2 +-
internal/operator-controller/authentication/synthetic.go | 3 +--
internal/operator-controller/authentication/synthetic_test.go | 3 +--
4 files changed, 4 insertions(+), 6 deletions(-)
diff --git a/hack/demo/resources/synthetic-user-perms/argocd-clusterextension.yaml b/hack/demo/resources/synthetic-user-perms/argocd-clusterextension.yaml
index a8dc8309f..7eb5a7082 100644
--- a/hack/demo/resources/synthetic-user-perms/argocd-clusterextension.yaml
+++ b/hack/demo/resources/synthetic-user-perms/argocd-clusterextension.yaml
@@ -10,4 +10,4 @@ spec:
sourceType: Catalog
catalog:
packageName: argocd-operator
- version: 0.6.0
\ No newline at end of file
+ version: 0.6.0
diff --git a/hack/demo/resources/synthetic-user-perms/cegroup-admin-binding.yaml b/hack/demo/resources/synthetic-user-perms/cegroup-admin-binding.yaml
index 069d0ea7a..d0ab570f7 100644
--- a/hack/demo/resources/synthetic-user-perms/cegroup-admin-binding.yaml
+++ b/hack/demo/resources/synthetic-user-perms/cegroup-admin-binding.yaml
@@ -8,4 +8,4 @@ roleRef:
name: cluster-admin
subjects:
- kind: Group
- name: "olm:clusterextensions"
\ No newline at end of file
+ name: "olm:clusterextensions"
diff --git a/internal/operator-controller/authentication/synthetic.go b/internal/operator-controller/authentication/synthetic.go
index a07a88d91..710f2885e 100644
--- a/internal/operator-controller/authentication/synthetic.go
+++ b/internal/operator-controller/authentication/synthetic.go
@@ -9,13 +9,12 @@ import (
)
func syntheticUserName(ext ocv1.ClusterExtension) string {
- return fmt.Sprintf("olm:clusterextensions:%s", ext.Name)
+ return fmt.Sprintf("olm:clusterextension:%s", ext.Name)
}
func syntheticGroups(_ ocv1.ClusterExtension) []string {
return []string{
"olm:clusterextensions",
- "system:authenticated",
}
}
diff --git a/internal/operator-controller/authentication/synthetic_test.go b/internal/operator-controller/authentication/synthetic_test.go
index f27025444..2e3f17a07 100644
--- a/internal/operator-controller/authentication/synthetic_test.go
+++ b/internal/operator-controller/authentication/synthetic_test.go
@@ -16,10 +16,9 @@ func TestSyntheticImpersonationConfig(t *testing.T) {
Name: "my-ext",
},
})
- require.Equal(t, "olm:clusterextensions:my-ext", config.UserName)
+ require.Equal(t, "olm:clusterextension:my-ext", config.UserName)
require.Equal(t, []string{
"olm:clusterextensions",
- "system:authenticated",
}, config.Groups)
require.Empty(t, config.UID)
require.Empty(t, config.Extra)
From 626ba75a471269bf4b43b7b48098373642de4cd9 Mon Sep 17 00:00:00 2001
From: Per Goncalves da Silva <pegoncal@redhat.com>
Date: Tue, 6 May 2025 15:49:37 +0200
Subject: [PATCH 13/13] Move FG check to main
Signed-off-by: Per Goncalves da Silva <pegoncal@redhat.com>
---
cmd/operator-controller/main.go | 5 +-
.../operator-controller/action/restconfig.go | 67 +++---
.../action/restconfig_test.go | 195 +++++++++++++-----
3 files changed, 179 insertions(+), 88 deletions(-)
diff --git a/cmd/operator-controller/main.go b/cmd/operator-controller/main.go
index 282cb462f..592d52507 100644
--- a/cmd/operator-controller/main.go
+++ b/cmd/operator-controller/main.go
@@ -304,7 +304,10 @@ func run() error {
return err
}
tokenGetter := authentication.NewTokenGetter(coreClient, authentication.WithExpirationDuration(1*time.Hour))
- clientRestConfigMapper := action.ClusterExtensionUserRestConfigMapper(tokenGetter)
+ clientRestConfigMapper := action.ServiceAccountRestConfigMapper(tokenGetter)
+ if features.OperatorControllerFeatureGate.Enabled(features.SyntheticPermissions) {
+ clientRestConfigMapper = action.SyntheticUserRestConfigMapper(clientRestConfigMapper)
+ }
cfgGetter, err := helmclient.NewActionConfigGetter(mgr.GetConfig(), mgr.GetRESTMapper(),
helmclient.StorageDriverMapper(action.ChunkedStorageDriverMapper(coreClient, mgr.GetAPIReader(), cfg.systemNamespace)),
diff --git a/internal/operator-controller/action/restconfig.go b/internal/operator-controller/action/restconfig.go
index b83696621..05e25f707 100644
--- a/internal/operator-controller/action/restconfig.go
+++ b/internal/operator-controller/action/restconfig.go
@@ -2,6 +2,7 @@ package action
import (
"context"
+ "fmt"
"net/http"
"k8s.io/apimachinery/pkg/types"
@@ -11,38 +12,22 @@ import (
ocv1 "github.com/operator-framework/operator-controller/api/v1"
"github.com/operator-framework/operator-controller/internal/operator-controller/authentication"
- "github.com/operator-framework/operator-controller/internal/operator-controller/features"
)
const syntheticServiceAccountName = "olm.synthetic-user"
-type clusterExtensionRestConfigMapper struct {
- saRestConfigMapper func(ctx context.Context, o client.Object, c *rest.Config) (*rest.Config, error)
- synthUserRestConfigMapper func(ctx context.Context, o client.Object, c *rest.Config) (*rest.Config, error)
-}
-
-func (m *clusterExtensionRestConfigMapper) mapper() func(ctx context.Context, o client.Object, c *rest.Config) (*rest.Config, error) {
- synthAuthFeatureEnabled := features.OperatorControllerFeatureGate.Enabled(features.SyntheticPermissions)
+// SyntheticUserRestConfigMapper returns an AuthConfigMapper that that impersonates synthetic users and groups for Object o.
+// o is expected to be a ClusterExtension. If the service account defined in o is different from 'olm.synthetic-user', the
+// defaultAuthMapper will be used
+func SyntheticUserRestConfigMapper(defaultAuthMapper func(ctx context.Context, o client.Object, c *rest.Config) (*rest.Config, error)) 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 synthAuthFeatureEnabled && cExt.Spec.ServiceAccount.Name == syntheticServiceAccountName {
- return m.synthUserRestConfigMapper(ctx, o, c)
+ cExt, err := validate(o, c)
+ if err != nil {
+ return nil, err
+ }
+ if cExt.Spec.ServiceAccount.Name != syntheticServiceAccountName {
+ return defaultAuthMapper(ctx, cExt, c)
}
- return m.saRestConfigMapper(ctx, o, c)
- }
-}
-
-func ClusterExtensionUserRestConfigMapper(tokenGetter *authentication.TokenGetter) func(ctx context.Context, o client.Object, c *rest.Config) (*rest.Config, error) {
- m := &clusterExtensionRestConfigMapper{
- saRestConfigMapper: serviceAccountRestConfigMapper(tokenGetter),
- synthUserRestConfigMapper: syntheticUserRestConfigMapper(),
- }
- return m.mapper()
-}
-
-func syntheticUserRestConfigMapper() 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)
cc := rest.CopyConfig(c)
cc.Wrap(func(rt http.RoundTripper) http.RoundTripper {
return transport.NewImpersonatingRoundTripper(authentication.SyntheticImpersonationConfig(*cExt), rt)
@@ -51,21 +36,39 @@ func syntheticUserRestConfigMapper() func(ctx context.Context, o client.Object,
}
}
-func serviceAccountRestConfigMapper(tokenGetter *authentication.TokenGetter) func(ctx context.Context, o client.Object, c *rest.Config) (*rest.Config, error) {
+// ServiceAccountRestConfigMapper returns an AuthConfigMapper scoped to the service account defined in o, which is expected to
+// be a ClusterExtension
+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)
- saKey := types.NamespacedName{
- Name: cExt.Spec.ServiceAccount.Name,
- Namespace: cExt.Spec.Namespace,
+ cExt, err := validate(o, c)
+ if err != nil {
+ return nil, err
}
saConfig := rest.AnonymousClientConfig(c)
saConfig.Wrap(func(rt http.RoundTripper) http.RoundTripper {
return &authentication.TokenInjectingRoundTripper{
Tripper: rt,
TokenGetter: tokenGetter,
- Key: saKey,
+ Key: types.NamespacedName{
+ Name: cExt.Spec.ServiceAccount.Name,
+ Namespace: cExt.Spec.Namespace,
+ },
}
})
return saConfig, nil
}
}
+
+func validate(o client.Object, c *rest.Config) (*ocv1.ClusterExtension, error) {
+ if c == nil {
+ return nil, fmt.Errorf("rest config is nil")
+ }
+ if o == nil {
+ return nil, fmt.Errorf("object is nil")
+ }
+ cExt, ok := o.(*ocv1.ClusterExtension)
+ if !ok {
+ return nil, fmt.Errorf("object is not a ClusterExtension")
+ }
+ return cExt, nil
+}
diff --git a/internal/operator-controller/action/restconfig_test.go b/internal/operator-controller/action/restconfig_test.go
index 94e9bf807..4c9f78671 100644
--- a/internal/operator-controller/action/restconfig_test.go
+++ b/internal/operator-controller/action/restconfig_test.go
@@ -1,92 +1,177 @@
-package action
+package action_test
import (
"context"
+ "errors"
+ "net/http"
"testing"
"github.com/stretchr/testify/require"
+ corev1 "k8s.io/api/core/v1"
+ metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+ "k8s.io/apimachinery/pkg/types"
"k8s.io/client-go/rest"
- featuregatetesting "k8s.io/component-base/featuregate/testing"
"sigs.k8s.io/controller-runtime/pkg/client"
ocv1 "github.com/operator-framework/operator-controller/api/v1"
- "github.com/operator-framework/operator-controller/internal/operator-controller/features"
+ "github.com/operator-framework/operator-controller/internal/operator-controller/action"
+ "github.com/operator-framework/operator-controller/internal/operator-controller/authentication"
)
-const (
- saAccountWrapper = "service account wrapper"
- synthUserWrapper = "synthetic user wrapper"
-)
-
-func fakeRestConfigWrapper() clusterExtensionRestConfigMapper {
- // The rest config's host field is artificially used to differentiate between the wrappers
- return clusterExtensionRestConfigMapper{
- saRestConfigMapper: func(ctx context.Context, o client.Object, c *rest.Config) (*rest.Config, error) {
- return &rest.Config{
- Host: saAccountWrapper,
- }, nil
- },
- synthUserRestConfigMapper: func(ctx context.Context, o client.Object, c *rest.Config) (*rest.Config, error) {
- return &rest.Config{
- Host: synthUserWrapper,
- }, nil
+func Test_ServiceAccountRestConfigMapper(t *testing.T) {
+ for _, tc := range []struct {
+ description string
+ obj client.Object
+ cfg *rest.Config
+ expectedError error
+ }{
+ {
+ description: "return error if object is nil",
+ cfg: &rest.Config{},
+ expectedError: errors.New("object is nil"),
+ }, {
+ description: "return error if cfg is nil",
+ obj: &ocv1.ClusterExtension{},
+ expectedError: errors.New("rest config is nil"),
+ }, {
+ description: "return error if object is not a ClusterExtension",
+ obj: &corev1.Secret{},
+ cfg: &rest.Config{},
+ expectedError: errors.New("object is not a ClusterExtension"),
+ }, {
+ description: "succeeds if object is not a ClusterExtension",
+ obj: &ocv1.ClusterExtension{
+ ObjectMeta: metav1.ObjectMeta{
+ Name: "my-clusterextension",
+ },
+ Spec: ocv1.ClusterExtensionSpec{
+ ServiceAccount: ocv1.ServiceAccountReference{
+ Name: "my-service-account",
+ },
+ Namespace: "my-namespace",
+ },
+ },
+ cfg: &rest.Config{},
},
+ } {
+ t.Run(tc.description, func(t *testing.T) {
+ tokenGetter := &authentication.TokenGetter{}
+ saMapper := action.ServiceAccountRestConfigMapper(tokenGetter)
+ actualCfg, err := saMapper(context.Background(), tc.obj, tc.cfg)
+ if tc.expectedError != nil {
+ require.Nil(t, actualCfg)
+ require.EqualError(t, err, tc.expectedError.Error())
+ } else {
+ require.NoError(t, err)
+ transport, err := rest.TransportFor(actualCfg)
+ require.NoError(t, err)
+ require.NotNil(t, transport)
+ tokenInjectionRoundTripper, ok := transport.(*authentication.TokenInjectingRoundTripper)
+ require.True(t, ok)
+ require.Equal(t, tokenGetter, tokenInjectionRoundTripper.TokenGetter)
+ require.Equal(t, types.NamespacedName{Name: "my-service-account", Namespace: "my-namespace"}, tokenInjectionRoundTripper.Key)
+ }
+ })
}
}
-func TestMapper_SyntheticPermissionsEnabled(t *testing.T) {
- featuregatetesting.SetFeatureGateDuringTest(t, features.OperatorControllerFeatureGate, features.SyntheticPermissions, true)
-
+func Test_SyntheticUserRestConfigMapper_Fails(t *testing.T) {
for _, tc := range []struct {
- description string
- serviceAccountName string
- expectedMapper string
- fgEnabled bool
+ description string
+ obj client.Object
+ cfg *rest.Config
+ expectedError error
}{
{
- description: "user service account wrapper if extension service account is _not_ called olm.synthetic-user",
- serviceAccountName: "not.olm.synthetic-user",
- expectedMapper: saAccountWrapper,
- fgEnabled: true,
+ description: "return error if object is nil",
+ cfg: &rest.Config{},
+ expectedError: errors.New("object is nil"),
}, {
- description: "user synthetic user wrapper is extension service account is called olm.synthetic-user",
- serviceAccountName: "olm.synthetic-user",
- expectedMapper: synthUserWrapper,
- fgEnabled: true,
+ description: "return error if cfg is nil",
+ obj: &ocv1.ClusterExtension{},
+ expectedError: errors.New("rest config is nil"),
+ }, {
+ description: "return error if object is not a ClusterExtension",
+ obj: &corev1.Secret{},
+ cfg: &rest.Config{},
+ expectedError: errors.New("object is not a ClusterExtension"),
},
} {
t.Run(tc.description, func(t *testing.T) {
- m := fakeRestConfigWrapper()
- mapper := m.mapper()
- ext := &ocv1.ClusterExtension{
- Spec: ocv1.ClusterExtensionSpec{
- ServiceAccount: ocv1.ServiceAccountReference{
- Name: tc.serviceAccountName,
- },
- },
+ tokenGetter := &authentication.TokenGetter{}
+ saMapper := action.ServiceAccountRestConfigMapper(tokenGetter)
+ actualCfg, err := saMapper(context.Background(), tc.obj, tc.cfg)
+ if tc.expectedError != nil {
+ require.Nil(t, actualCfg)
+ require.EqualError(t, err, tc.expectedError.Error())
+ } else {
+ require.NoError(t, err)
+ transport, err := rest.TransportFor(actualCfg)
+ require.NoError(t, err)
+ require.NotNil(t, transport)
+ tokenInjectionRoundTripper, ok := transport.(*authentication.TokenInjectingRoundTripper)
+ require.True(t, ok)
+ require.Equal(t, tokenGetter, tokenInjectionRoundTripper.TokenGetter)
+ require.Equal(t, types.NamespacedName{Name: "my-service-account", Namespace: "my-namespace"}, tokenInjectionRoundTripper.Key)
}
- cfg, err := mapper(context.Background(), ext, &rest.Config{})
- require.NoError(t, err)
-
- // The rest config's host field is artificially used to differentiate between the wrappers
- require.Equal(t, tc.expectedMapper, cfg.Host)
})
}
}
+func Test_SyntheticUserRestConfigMapper_UsesDefaultConfigMapper(t *testing.T) {
+ isDefaultRequestMapperUsed := false
+ defaultServiceMapper := func(ctx context.Context, o client.Object, c *rest.Config) (*rest.Config, error) {
+ isDefaultRequestMapperUsed = true
+ return c, nil
+ }
+ syntheticAuthServiceMapper := action.SyntheticUserRestConfigMapper(defaultServiceMapper)
+ obj := &ocv1.ClusterExtension{
+ ObjectMeta: metav1.ObjectMeta{
+ Name: "my-clusterextension",
+ },
+ Spec: ocv1.ClusterExtensionSpec{
+ ServiceAccount: ocv1.ServiceAccountReference{
+ Name: "my-service-account",
+ },
+ Namespace: "my-namespace",
+ },
+ }
+ actualCfg, err := syntheticAuthServiceMapper(context.Background(), obj, &rest.Config{})
+ require.NoError(t, err)
+ require.NotNil(t, actualCfg)
+ require.True(t, isDefaultRequestMapperUsed)
+}
-func TestMapper_SyntheticPermissionsDisabled(t *testing.T) {
- m := fakeRestConfigWrapper()
- mapper := m.mapper()
- ext := &ocv1.ClusterExtension{
+func Test_SyntheticUserRestConfigMapper_UsesSyntheticAuthMapper(t *testing.T) {
+ syntheticAuthServiceMapper := action.SyntheticUserRestConfigMapper(func(ctx context.Context, o client.Object, c *rest.Config) (*rest.Config, error) {
+ return c, nil
+ })
+ obj := &ocv1.ClusterExtension{
+ ObjectMeta: metav1.ObjectMeta{
+ Name: "my-clusterextension",
+ },
Spec: ocv1.ClusterExtensionSpec{
ServiceAccount: ocv1.ServiceAccountReference{
Name: "olm.synthetic-user",
},
+ Namespace: "my-namespace",
},
}
- cfg, err := mapper(context.Background(), ext, &rest.Config{})
+ actualCfg, err := syntheticAuthServiceMapper(context.Background(), obj, &rest.Config{})
require.NoError(t, err)
+ require.NotNil(t, actualCfg)
+
+ // test that the impersonation headers are appropriately injected into the request
+ // by wrapping a fake round tripper around the returned configurations transport
+ // nolint:bodyclose
+ _, _ = actualCfg.WrapTransport(fakeRoundTripper(func(req *http.Request) (*http.Response, error) {
+ require.Equal(t, "olm:clusterextension:my-clusterextension", req.Header.Get("Impersonate-User"))
+ require.Equal(t, "olm:clusterextensions", req.Header.Get("Impersonate-Group"))
+ return &http.Response{}, nil
+ })).RoundTrip(&http.Request{})
+}
+
+type fakeRoundTripper func(req *http.Request) (*http.Response, error)
- // The rest config's host field is artificially used to differentiate between the wrappers
- require.Equal(t, saAccountWrapper, cfg.Host)
+func (f fakeRoundTripper) RoundTrip(request *http.Request) (*http.Response, error) {
+ return f(request)
}