Skip to content

Commit

Permalink
UPSTREAM: <carry>: protect pull secret annotation on admission
Browse files Browse the repository at this point in the history
  • Loading branch information
@sanchezl
sanchezl committed Jul 25, 2024
1 parent 32f817d commit 026dbc0
Show file tree
Hide file tree
Showing 4 changed files with 332 additions and 0 deletions.
3 changes: 3 additions & 0 deletions openshift-kube-apiserver/admission/admissionenablement/register.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -44,6 +45,7 @@ func RegisterOpenshiftKubeAdmissionPlugins(plugins *admission.Plugins) {
externalipranger.RegisterExternalIP(plugins)
restrictedendpoints.RegisterRestrictedEndpoints(plugins)
csiinlinevolumesecurity.Register(plugins)
internalregistry.Register(plugins)
}

var (
Expand Down Expand Up @@ -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
Expand Down
7 changes: 7 additions & 0 deletions openshift-kube-apiserver/admission/internalregistry/001.log
View file
Original file line number Diff line number Diff line change
@@ -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"]}}

73 changes: 73 additions & 0 deletions openshift-kube-apiserver/admission/internalregistry/admission.go
View file
Original file line number Diff line number Diff line change
@@ -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
}
249 changes: 249 additions & 0 deletions openshift-kube-apiserver/admission/internalregistry/admission_test.go
View file
Original file line number Diff line number Diff line change
@@ -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
}
}

0 comments on commit 026dbc0

Please sign in to comment.