diff --git a/openshift-kube-apiserver/admission/admissionenablement/register.go b/openshift-kube-apiserver/admission/admissionenablement/register.go index 296c593cb7c21..4d4e2b229e0e2 100644 --- a/openshift-kube-apiserver/admission/admissionenablement/register.go +++ b/openshift-kube-apiserver/admission/admissionenablement/register.go @@ -6,6 +6,7 @@ import ( "k8s.io/apiserver/pkg/admission/plugin/resourcequota" mutatingwebhook "k8s.io/apiserver/pkg/admission/plugin/webhook/mutating" "k8s.io/kubernetes/openshift-kube-apiserver/admission/autoscaling/mixedcpus" + "k8s.io/kubernetes/openshift-kube-apiserver/admission/internalregistry" "github.com/openshift/apiserver-library-go/pkg/admission/imagepolicy" imagepolicyapiv1 "github.com/openshift/apiserver-library-go/pkg/admission/imagepolicy/apis/imagepolicy/v1" @@ -44,6 +45,7 @@ func RegisterOpenshiftKubeAdmissionPlugins(plugins *admission.Plugins) { externalipranger.RegisterExternalIP(plugins) restrictedendpoints.RegisterRestrictedEndpoints(plugins) csiinlinevolumesecurity.Register(plugins) + internalregistry.Register(plugins) } var ( @@ -77,6 +79,7 @@ var ( csiinlinevolumesecurity.PluginName, // "storage.openshift.io/CSIInlineVolumeSecurity" managednode.PluginName, // "autoscaling.openshift.io/ManagedNode" mixedcpus.PluginName, // "autoscaling.openshift.io/MixedCPUs" + internalregistry.PluginName, // "registry.openshift.io/ProtectAnnotation" } // openshiftAdmissionPluginsForKubeAfterResourceQuota are the plugins to add after ResourceQuota plugin diff --git a/openshift-kube-apiserver/admission/internalregistry/001.log b/openshift-kube-apiserver/admission/internalregistry/001.log new file mode 100644 index 0000000000000..0eaed85553c9f --- /dev/null +++ b/openshift-kube-apiserver/admission/internalregistry/001.log @@ -0,0 +1,7 @@ +I0719 16:39:38.407880 14 admission.go:50] "##### Protected annotation changed" ns="openshift-operators" sa="deployer" annotation="openshift.io/internal-registry-pull-secret-ref" user {"Name":"sanchezl","UID":"25adb042-f739-4f77-864f-6d1e4828e515","Groups":["system:authenticated:oauth","system:authenticated"],"Extra":{"scopes.authorization.openshift.io":["user:full"]}} +I0719 16:40:41.682039 14 admission.go:50] "##### Protected annotation changed" ns="openshift-operators" sa="my-cluster-zookeeper" annotation="openshift.io/internal-registry-pull-secret-ref" user= {"Name":"system:serviceaccount:openshift-operators:strimzi-cluster-operator","UID":"7397fb04-4223-4265-980a-38a76305036e","Groups":["system:serviceaccounts","system:serviceaccounts:openshift-operators","system:authenticated"],"Extra":{"authentication.kubernetes.io/credential-id":["JTI=ec66e3ab-ea85-44fb-8d97-e051393b73bd"],"authentication.kubernetes.io/node-name":["ip-10-0-34-165.us-west-1.compute.internal"],"authentication.kubernetes.io/node-uid":["296a0066-8ed1-49dd-9efe-faf94769a516"],"authentication.kubernetes.io/pod-name":["amq-streams-cluster-operator-v2.7.0-2-789f6448fc-tnz55"],"authentication.kubernetes.io/pod-uid":["aee5badf-ba03-4e40-a1e6-a2460e5efe7d"]}} +I0719 16:40:43.468362 14 admission.go:50] "##### Protected annotation changed" ns="openshift-operators" sa="my-cluster-kafka" annotation="openshift.io/internal-registry-pull-secret-ref" user= {"Name":"system:serviceaccount:openshift-operators:strimzi-cluster-operator","UID":"7397fb04-4223-4265-980a-38a76305036e","Groups":["system:serviceaccounts","system:serviceaccounts:openshift-operators","system:authenticated"],"Extra":{"authentication.kubernetes.io/credential-id":["JTI=ec66e3ab-ea85-44fb-8d97-e051393b73bd"],"authentication.kubernetes.io/node-name":["ip-10-0-34-165.us-west-1.compute.internal"],"authentication.kubernetes.io/node-uid":["296a0066-8ed1-49dd-9efe-faf94769a516"],"authentication.kubernetes.io/pod-name":["amq-streams-cluster-operator-v2.7.0-2-789f6448fc-tnz55"],"authentication.kubernetes.io/pod-uid":["aee5badf-ba03-4e40-a1e6-a2460e5efe7d"]}} +I0719 16:41:27.389938 14 admission.go:50] "##### Protected annotation changed" ns="openshift-operators" sa="my-cluster-zookeeper" annotation="openshift.io/internal-registry-pull-secret-ref" user= {"Name":"system:serviceaccount:openshift-operators:strimzi-cluster-operator","UID":"7397fb04-4223-4265-980a-38a76305036e","Groups":["system:serviceaccounts","system:serviceaccounts:openshift-operators","system:authenticated"],"Extra":{"authentication.kubernetes.io/credential-id":["JTI=2b2beee2-a445-4e8c-9bc5-4f927351f35c"],"authentication.kubernetes.io/node-name":["ip-10-0-34-165.us-west-1.compute.internal"],"authentication.kubernetes.io/node-uid":["296a0066-8ed1-49dd-9efe-faf94769a516"],"authentication.kubernetes.io/pod-name":["amq-streams-cluster-operator-v2.7.0-2-789f6448fc-tnz55"],"authentication.kubernetes.io/pod-uid":["aee5badf-ba03-4e40-a1e6-a2460e5efe7d"]}} +I0719 16:41:28.682630 14 admission.go:50] "##### Protected annotation changed" ns="openshift-operators" sa="my-cluster-kafka" annotation="openshift.io/internal-registry-pull-secret-ref" user= {"Name":"system:serviceaccount:openshift-operators:strimzi-cluster-operator","UID":"7397fb04-4223-4265-980a-38a76305036e","Groups":["system:serviceaccounts","system:serviceaccounts:openshift-operators","system:authenticated"],"Extra":{"authentication.kubernetes.io/credential-id":["JTI=2b2beee2-a445-4e8c-9bc5-4f927351f35c"],"authentication.kubernetes.io/node-name":["ip-10-0-34-165.us-west-1.compute.internal"],"authentication.kubernetes.io/node-uid":["296a0066-8ed1-49dd-9efe-faf94769a516"],"authentication.kubernetes.io/pod-name":["amq-streams-cluster-operator-v2.7.0-2-789f6448fc-tnz55"],"authentication.kubernetes.io/pod-uid":["aee5badf-ba03-4e40-a1e6-a2460e5efe7d"]}} +I0719 16:41:29.677529 14 admission.go:50] "##### Protected annotation changed" ns="openshift-operators" sa="my-cluster-entity-operator" annotation="openshift.io/internal-registry-pull-secret-ref" user={"Name":"system:serviceaccount:openshift-operators:strimzi-cluster-operator","UID":"7397fb04-4223-4265-980a-38a76305036e","Groups":["system:serviceaccounts","system:serviceaccounts:openshift-operators","system:authenticated"],"Extra":{"authentication.kubernetes.io/credential-id":["JTI=2b2beee2-a445-4e8c-9bc5-4f927351f35c"],"authentication.kubernetes.io/node-name":["ip-10-0-34-165.us-west-1.compute.internal"],"authentication.kubernetes.io/node-uid":["296a0066-8ed1-49dd-9efe-faf94769a516"],"authentication.kubernetes.io/pod-name":["amq-streams-cluster-operator-v2.7.0-2-789f6448fc-tnz55"],"authentication.kubernetes.io/pod-uid":["aee5badf-ba03-4e40-a1e6-a2460e5efe7d"]}} + diff --git a/openshift-kube-apiserver/admission/internalregistry/admission.go b/openshift-kube-apiserver/admission/internalregistry/admission.go new file mode 100644 index 0000000000000..349fe6cf32a33 --- /dev/null +++ b/openshift-kube-apiserver/admission/internalregistry/admission.go @@ -0,0 +1,73 @@ +package internalregistry + +import ( + "context" + "fmt" + "io" + + "golang.org/x/exp/slices" + "k8s.io/apimachinery/pkg/api/meta" + "k8s.io/apiserver/pkg/admission" +) + +const PluginName = "registry.openshift.io/ProtectAnnotation" + +func Register(plugins *admission.Plugins) { + plugins.Register(PluginName, func(config io.Reader) (admission.Interface, error) { + return protectAnnotation{Handler: admission.NewHandler(admission.Update)}, nil + }) +} + +// protectAnnotation plugin prevents annotations set by OCM from being overwritten by other service accounts. +type protectAnnotation struct { + *admission.Handler +} + +const protectedAnnotationKey = "openshift.io/internal-registry-pull-secret-ref" + +var allowedServiceAccounts = []string{ + "system:serviceaccounts:openshift-infra", + "system:serviceaccounts:openshift-controller-manager", + "system:serviceaccounts:openshift-controller-manager-operator", +} + +func (m protectAnnotation) Validate(ctx context.Context, a admission.Attributes, o admission.ObjectInterfaces) error { + if a.GetResource().Resource != "serviceaccounts" { + return nil + } + if !slices.Contains(a.GetUserInfo().GetGroups(), "system:serviceaccounts") { + // not a service account, allow + return nil + } + if slices.IndexFunc(a.GetUserInfo().GetGroups(), func(s string) bool { + return slices.Contains(allowedServiceAccounts, s) + }) > 0 { + return nil + } + changed, err := annotationChanged(a, protectedAnnotationKey) + if err != nil { + return err + } + if !changed { + return nil + } + return admission.NewForbidden(a, fmt.Errorf("'%s' annotation can only be changed by openshift-controller-manager", protectedAnnotationKey)) +} + +func annotationChanged(a admission.Attributes, key string) (bool, error) { + before, err := meta.Accessor(a.GetOldObject()) + if err != nil { + return false, err + } + after, err := meta.Accessor(a.GetObject()) + if err != nil { + return false, err + } + valueBefore, keyExistedBefore := before.GetAnnotations()[key] + valueAfter, keyExistsAfter := after.GetAnnotations()[key] + if (keyExistedBefore == keyExistsAfter) && (valueBefore == valueAfter) { + // annotation was not changed + return false, nil + } + return true, nil +} diff --git a/openshift-kube-apiserver/admission/internalregistry/admission_test.go b/openshift-kube-apiserver/admission/internalregistry/admission_test.go new file mode 100644 index 0000000000000..af36fe3d44404 --- /dev/null +++ b/openshift-kube-apiserver/admission/internalregistry/admission_test.go @@ -0,0 +1,249 @@ +package internalregistry + +import ( + "context" + "testing" + + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/meta" + "k8s.io/apimachinery/pkg/api/meta/testrestmapper" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apiserver/pkg/admission" + auditinternal "k8s.io/apiserver/pkg/apis/audit" + "k8s.io/apiserver/pkg/authentication/user" +) + +func TestProtectAnnotationValidate(t *testing.T) { + + testCases := []struct { + name string + attributes admission.Attributes + wantError bool + }{ + { + name: "serviceaccount edit", + attributes: attributes( + withOldObject(sa(withProtectedAnnotationValue("before"))), + withObject(sa(withProtectedAnnotationValue("after"))), + withUser("system:serviceaccounts", "foo"), + ), + wantError: true, + }, + { + name: "serviceaccount delete", + attributes: attributes( + withOldObject(sa(withProtectedAnnotationValue("before"))), + withObject(sa()), + withUser("system:serviceaccounts", "foo"), + ), + wantError: true, + }, + { + name: "serviceaccount add", + attributes: attributes( + withObject(sa(withProtectedAnnotationValue("after"))), + withOldObject(sa()), + withUser("system:serviceaccounts", "foo"), + ), + wantError: true, + }, + { + name: "serviceaccount no change", + attributes: attributes( + withOldObject(sa(withProtectedAnnotationValue("before"), withAnnotation("foo", "before"))), + withObject(sa(withProtectedAnnotationValue("before"), withAnnotation("foo", "after"))), + withUser("system:serviceaccounts", "foo"), + ), + wantError: false, + }, + { + name: "ocm serviceaccount edit", + attributes: attributes( + withOldObject(sa(withProtectedAnnotationValue("before"))), + withObject(sa(withProtectedAnnotationValue("after"))), + withUser("system:serviceaccounts", "system:serviceaccounts:openshift-infra"), + ), + wantError: false, + }, + { + name: "ocm serviceaccount delete", + attributes: attributes( + withOldObject(sa(withProtectedAnnotationValue("before"))), + withObject(sa()), + withUser("system:serviceaccounts", "system:serviceaccounts:openshift-infra"), + ), + wantError: false, + }, + { + name: "ocm serviceaccount add", + attributes: attributes( + withObject(sa(withProtectedAnnotationValue("after"))), + withOldObject(sa()), + withUser("system:serviceaccounts", "system:serviceaccounts:openshift-infra"), + ), + wantError: false, + }, + { + name: "ocm serviceaccount no change", + attributes: attributes( + withOldObject(sa(withProtectedAnnotationValue("before"), withAnnotation("foo", "before"))), + withObject(sa(withProtectedAnnotationValue("before"), withAnnotation("foo", "after"))), + withUser("system:serviceaccounts", "system:serviceaccounts:openshift-infra"), + ), + wantError: false, + }, + { + name: "user edit", + attributes: attributes( + withOldObject(sa(withProtectedAnnotationValue("before"))), + withObject(sa(withProtectedAnnotationValue("after"))), + withUser("system:authenticated"), + ), + wantError: false, + }, + { + name: "not a service account", + attributes: attributes( + withOldObject(pod(withProtectedAnnotationValue("before"))), + withObject(pod(withProtectedAnnotationValue("after"))), + withUser("system:serviceaccounts"), + ), + wantError: false, + }, + } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + err := protectAnnotation{}.Validate(context.TODO(), tc.attributes, nil) + if (err != nil) != tc.wantError { + if tc.wantError { + t.Fatal("expected error") + } + t.Fatalf("unexpected error: %v", err) + } + }) + } + +} + +type attributesRecord struct { + kind schema.GroupVersionKind + namespace string + name string + resource schema.GroupVersionResource + subresource string + operation admission.Operation + options runtime.Object + dryRun bool + object runtime.Object + oldObject runtime.Object + userInfo user.Info +} + +func (a attributesRecord) GetName() string { return a.name } +func (a attributesRecord) GetNamespace() string { return a.namespace } +func (a attributesRecord) GetResource() schema.GroupVersionResource { return a.resource } +func (a attributesRecord) GetSubresource() string { return a.subresource } +func (a attributesRecord) GetOperation() admission.Operation { return a.operation } +func (a attributesRecord) GetOperationOptions() runtime.Object { return a.options } +func (a attributesRecord) IsDryRun() bool { return a.dryRun } +func (a attributesRecord) GetObject() runtime.Object { return a.object } +func (a attributesRecord) GetOldObject() runtime.Object { return a.oldObject } +func (a attributesRecord) GetKind() schema.GroupVersionKind { return a.kind } +func (a attributesRecord) GetUserInfo() user.Info { return a.userInfo } +func (a attributesRecord) AddAnnotation(key, value string) error { panic("implement me") } +func (a attributesRecord) AddAnnotationWithLevel(key, value string, level auditinternal.Level) error { + panic("implement me") +} +func (a attributesRecord) GetReinvocationContext() admission.ReinvocationContext { + panic("implement me") +} + +func attributes(opts ...func(*attributesRecord)) *attributesRecord { + a := &attributesRecord{ + namespace: "test-ns", + operation: admission.Update, + options: &metav1.UpdateOptions{}, + } + for _, opt := range opts { + opt(a) + } + return a +} + +func withObject(o runtime.Object) func(*attributesRecord) { + return func(record *attributesRecord) { + record.object = o + record.kind = o.GetObjectKind().GroupVersionKind() + record.resource = gvk2gvr(record.kind) + } +} + +func gvk2gvr(gvk schema.GroupVersionKind) schema.GroupVersionResource { + scheme := runtime.NewScheme() + corev1.AddToScheme(scheme) + mapper := testrestmapper.TestOnlyStaticRESTMapper(scheme) + mapping, err := mapper.RESTMapping(gvk.GroupKind()) + if err != nil { + panic(err) + } + return mapping.Resource +} + +func withOldObject(o runtime.Object) func(*attributesRecord) { + return func(record *attributesRecord) { + record.oldObject = o + } +} + +func withUser(g ...string) func(*attributesRecord) { + return func(record *attributesRecord) { + record.userInfo = &user.DefaultInfo{Name: "test", Groups: g} + } +} + +func pod(opts ...func(runtime.Object)) *corev1.Pod { + pod := &corev1.Pod{ + TypeMeta: metav1.TypeMeta{ + Kind: "Pod", + APIVersion: "v1", + }, + } + pod.Name = "test-pod" + for _, opt := range opts { + opt(pod) + } + return pod +} + +func sa(opts ...func(runtime.Object)) *corev1.ServiceAccount { + sa := &corev1.ServiceAccount{ + TypeMeta: metav1.TypeMeta{ + Kind: "ServiceAccount", + APIVersion: "v1", + }, + } + sa.Name = "test-sa" + for _, opt := range opts { + opt(sa) + } + return sa +} + +func withProtectedAnnotationValue(v string) func(object runtime.Object) { + return withAnnotation(protectedAnnotationKey, v) +} + +func withAnnotation(k, v string) func(object runtime.Object) { + return func(o runtime.Object) { + m, err := meta.Accessor(o) + if err != nil { + panic(err) + } + if m.GetAnnotations() == nil { + m.SetAnnotations(make(map[string]string)) + } + m.GetAnnotations()[k] = v + } +}