diff --git a/pkg/resources/core/v1/secret/mutator.go b/pkg/resources/core/v1/secret/mutator.go index 06f8d219a..1e854ed56 100644 --- a/pkg/resources/core/v1/secret/mutator.go +++ b/pkg/resources/core/v1/secret/mutator.go @@ -72,6 +72,13 @@ func (m *Mutator) MutatingWebhook(clientConfig admissionregistrationv1.WebhookCl mutatingWebhook := admission.NewDefaultMutatingWebhook(m, clientConfig, admissionregistrationv1.NamespacedScope, m.Operations()) mutatingWebhook.SideEffects = admission.Ptr(admissionregistrationv1.SideEffectClassNoneOnDryRun) mutatingWebhook.TimeoutSeconds = admission.Ptr(int32(15)) + mutatingWebhook.MatchConditions = []admissionregistrationv1.MatchCondition{ + { + Name: "filter-by-secret-type-cloud-credential", + Expression: `request.operation == 'DELETE' || (object != null && object.type == "provisioning.cattle.io/cloud-credential")`, + }, + } + return []admissionregistrationv1.MutatingWebhook{*mutatingWebhook} } @@ -82,7 +89,6 @@ func (m *Mutator) Admit(request *admission.Request) (*admissionv1.AdmissionRespo Allowed: true, }, nil } - listTrace := trace.New("secret Admit", trace.Field{Key: "user", Value: request.UserInfo.Username}) defer listTrace.LogIfLong(admission.SlowTraceDuration) @@ -101,12 +107,6 @@ func (m *Mutator) Admit(request *admission.Request) (*admissionv1.AdmissionRespo } func (m *Mutator) admitCreate(secret *corev1.Secret, request *admission.Request) (*admissionv1.AdmissionResponse, error) { - if secret.Type != "provisioning.cattle.io/cloud-credential" { - return &admissionv1.AdmissionResponse{ - Allowed: true, - }, nil - } - logrus.Debugf("[secret-mutation] adding creatorID %v to secret: %v", request.UserInfo.Username, secret.Name) newSecret := secret.DeepCopy() diff --git a/tests/integration/secret_test.go b/tests/integration/secret_test.go index 069f4552a..c8dfd38ef 100644 --- a/tests/integration/secret_test.go +++ b/tests/integration/secret_test.go @@ -6,31 +6,194 @@ import ( "github.com/rancher/webhook/pkg/resources/common" v1 "k8s.io/api/core/v1" + rbacv1 "k8s.io/api/rbac/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" ) -func (m *IntegrationSuite) TestSecret() { +func (m *IntegrationSuite) TestSecretMutations() { objGVK := schema.GroupVersionKind{ Group: "", Version: "v1", Kind: "Secret", } - validCreateObj := &v1.Secret{ + + tests := []struct { + name string + secret *v1.Secret + expectMutated bool + }{ + { + name: "Cloud credential secret is mutated", + secret: &v1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cloud-credential-secret", + Namespace: testNamespace, + }, + Type: v1.SecretType("provisioning.cattle.io/cloud-credential"), + }, + expectMutated: true, + }, + { + name: "Opaque secret is not mutated", + secret: &v1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-opaque-secret", + Namespace: testNamespace, + }, + Type: v1.SecretTypeOpaque, + StringData: map[string]string{ + "username": "myuser", + "password": "mypassword", + }, + }, + expectMutated: false, + }, + } + + for _, tt := range tests { + tt := tt + m.Run(tt.name, func() { + client, err := m.clientFactory.ForKind(objGVK) + m.Require().NoError(err, "Failed to create client") + + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + + result := &v1.Secret{} + err = client.Create(ctx, tt.secret.Namespace, tt.secret, result, metav1.CreateOptions{}) + m.NoError(err, "Error creating secret") + + if tt.expectMutated { + m.Contains(result.Annotations, common.CreatorIDAnn, "Expected secret to be mutated with creator annotation") + } else { + m.NotContains(result.Annotations, common.CreatorIDAnn, "Expected secret NOT to be mutated") + } + }) + } +} + +// TestSecretDeleteMutation tests the mutation of RoleBindings when a secret is deleted. +// 1. Intercepts secret deletion requests +// 2. Finds any RoleBindings owned by the secret being deleted +// 3. Checks if these RoleBindings grant access to Roles that provide permissions to the secret +// 4. If found, redacts these Roles to only retain the "delete" permission + +func (m *IntegrationSuite) TestSecretDeleteMutation() { + // Setup GVK for Secret + objGVK := schema.GroupVersionKind{ + Group: "", + Version: "v1", + Kind: "Secret", + } + + // Setup GVK for RoleBinding + rbGVK := schema.GroupVersionKind{ + Group: "rbac.authorization.k8s.io", + Version: "v1", + Kind: "RoleBinding", + } + + // Setup GVK for Role + roleGVK := schema.GroupVersionKind{ + Group: "rbac.authorization.k8s.io", + Version: "v1", + Kind: "Role", + } + + // 1. Create a client for each resource type + secretClient, err := m.clientFactory.ForKind(objGVK) + m.Require().NoError(err, "Failed to create secret client") + + roleClient, err := m.clientFactory.ForKind(roleGVK) + m.Require().NoError(err, "Failed to create role client") + + rbClient, err := m.clientFactory.ForKind(rbGVK) + m.Require().NoError(err, "Failed to create rolebinding client") + + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + + // 2. Create a test secret + testSecret := &v1.Secret{ ObjectMeta: metav1.ObjectMeta{ - Name: "test-cluster", + Name: "test-delete-secret", Namespace: testNamespace, }, - Type: v1.SecretType("provisioning.cattle.io/cloud-credential"), + Type: v1.SecretTypeOpaque, + StringData: map[string]string{ + "username": "testuser", + "password": "testpass", + }, } - client, err := m.clientFactory.ForKind(objGVK) - m.Require().NoError(err, "Failed to create client") - result := &v1.Secret{} - ctx, cancel := context.WithTimeout(context.Background(), time.Second*2) - defer cancel() - err = client.Create(ctx, validCreateObj.Namespace, validCreateObj, result, metav1.CreateOptions{}) - m.NoError(err, "Error returned during the creation of a valid Object") - m.Contains(result.Annotations, common.CreatorIDAnn) - err = client.Delete(ctx, validCreateObj.Namespace, validCreateObj.Name, metav1.DeleteOptions{}) - m.NoError(err, "Error returned during the deletion of a valid Object") + + secretResult := &v1.Secret{} + err = secretClient.Create(ctx, testSecret.Namespace, testSecret, secretResult, metav1.CreateOptions{}) + m.Require().NoError(err, "Failed to create test secret") + + // 3. Create a role that grants access to this secret + testRole := &rbacv1.Role{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-secret-role", + Namespace: testNamespace, + }, + Rules: []rbacv1.PolicyRule{ + { + APIGroups: []string{""}, + Resources: []string{"secrets"}, + ResourceNames: []string{testSecret.Name}, + Verbs: []string{"get", "update", "delete"}, + }, + }, + } + + roleResult := &rbacv1.Role{} + err = roleClient.Create(ctx, testRole.Namespace, testRole, roleResult, metav1.CreateOptions{}) + m.Require().NoError(err, "Failed to create test role") + + // 4. Create a rolebinding that references this role and is owned by the secret + testRoleBinding := &rbacv1.RoleBinding{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-secret-rolebinding", + Namespace: testNamespace, + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: "v1", + Kind: "Secret", + Name: testSecret.Name, + UID: secretResult.UID, + }, + }, + }, + RoleRef: rbacv1.RoleRef{ + APIGroup: "rbac.authorization.k8s.io", + Kind: "Role", + Name: testRole.Name, + }, + Subjects: []rbacv1.Subject{ + { + Kind: "User", + APIGroup: "rbac.authorization.k8s.io", + Name: "test-user", + }, + }, + } + + rbResult := &rbacv1.RoleBinding{} + err = rbClient.Create(ctx, testRoleBinding.Namespace, testRoleBinding, rbResult, metav1.CreateOptions{}) + m.Require().NoError(err, "Failed to create test rolebinding") + + // 5. Delete the secret - this should trigger the webhook's admitDelete function + err = secretClient.Delete(ctx, testSecret.Namespace, testSecret.Name, metav1.DeleteOptions{}) + m.Require().NoError(err, "Failed to delete test secret") + + // 6. Verify that the role was redacted to only include delete permission + updatedRole := &rbacv1.Role{} + err = roleClient.Get(ctx, testRole.Namespace, testRole.Name, updatedRole, metav1.GetOptions{}) + m.Require().NoError(err, "Failed to get updated role") + + // Check that the role now only has the delete verb + m.Require().Len(updatedRole.Rules, 1, "Role should have exactly one rule") + m.Require().Contains(updatedRole.Rules[0].Verbs, "delete", "Role should retain delete permission") + m.Require().Len(updatedRole.Rules[0].Verbs, 1, "Role should only have delete permission") }