Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

✨ auth: use synthetic user/group when service account is not defined #1816

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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 12 additions & 3 deletions api/v1/clusterextension_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
6 changes: 5 additions & 1 deletion api/v1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion cmd/operator-controller/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,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)),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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: |-
Expand Down Expand Up @@ -458,7 +467,6 @@ spec:
has(self.catalog) : !has(self.catalog)'
required:
- namespace
- serviceAccount
- source
type: object
status:
Expand Down
7 changes: 7 additions & 0 deletions config/base/operator-controller/rbac/role.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,13 @@ kind: ClusterRole
metadata:
name: manager-role
rules:
- apiGroups:
- ""
resources:
- groups
- users
verbs:
- impersonate
- apiGroups:
- ""
resources:
Expand Down
33 changes: 11 additions & 22 deletions config/samples/olm_v1_clusterextension.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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]
Expand Down Expand Up @@ -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:
Expand Down
2 changes: 1 addition & 1 deletion docs/api-reference/operator-controller-api-reference.md
Original file line number Diff line number Diff line change
Expand Up @@ -306,7 +306,7 @@ _Appears in:_
| Field | Description | Default | Validation |
| --- | --- | --- | --- |
| `namespace` _string_ | namespace is a reference to a Kubernetes namespace.<br />This is the namespace in which the provided ServiceAccount must exist.<br />It also designates the default namespace where namespace-scoped resources<br />for the extension are applied to the cluster.<br />Some extensions may contain namespace-scoped resources to be applied in other namespaces.<br />This namespace must exist.<br /><br />namespace is required, immutable, and follows the DNS label standard<br />as defined in [RFC 1123]. It must contain only lowercase alphanumeric characters or hyphens (-),<br />start and end with an alphanumeric character, and be no longer than 63 characters<br /><br />[RFC 1123]: https://tools.ietf.org/html/rfc1123 | | MaxLength: 63 <br />Required: \{\} <br /> |
| `serviceAccount` _[ServiceAccountReference](#serviceaccountreference)_ | serviceAccount is a reference to a ServiceAccount used to perform all interactions<br />with the cluster that are required to manage the extension.<br />The ServiceAccount must be configured with the necessary permissions to perform these interactions.<br />The ServiceAccount must exist in the namespace referenced in the spec.<br />serviceAccount is required. | | Required: \{\} <br /> |
| `serviceAccount` _[ServiceAccountReference](#serviceaccountreference)_ | serviceAccount is a reference to a ServiceAccount used to perform all interactions<br />with the cluster that are required to manage the extension.<br />The ServiceAccount must be configured with the necessary permissions to perform these interactions.<br />The ServiceAccount must exist in the namespace referenced in the spec.<br /><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. | | |

Expand Down
29 changes: 27 additions & 2 deletions internal/operator-controller/action/restconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
27 changes: 27 additions & 0 deletions internal/operator-controller/authentication/synthetic.go
Original file line number Diff line number Diff line change
@@ -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),
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,6 @@ func TestClusterExtensionSourceConfig(t *testing.T) {
},
},
Namespace: "default",
ServiceAccount: ocv1.ServiceAccountReference{
Name: "default",
},
}))
}
if tc.unionField == "" {
Expand All @@ -55,9 +52,6 @@ func TestClusterExtensionSourceConfig(t *testing.T) {
SourceType: tc.sourceType,
},
Namespace: "default",
ServiceAccount: ocv1.ServiceAccountReference{
Name: "default",
},
}))
}

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -374,7 +356,7 @@ func TestClusterExtensionAdmissionServiceAccount(t *testing.T) {
},
},
Namespace: "default",
ServiceAccount: ocv1.ServiceAccountReference{
ServiceAccount: &ocv1.ServiceAccountReference{
Name: tc.serviceAccount,
},
}))
Expand Down Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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=olm.operatorframework.io,resources=clustercatalogs,verbs=list;watch
Expand Down
Loading
Loading