Skip to content

Add e2e tests for cdp and deployment-service secret read permissions #9735

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

Draft
wants to merge 3 commits into
base: dev
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
70 changes: 70 additions & 0 deletions cluster/cluster.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -428,6 +428,76 @@ Resources:
KubernetesGroups:
- zalando:postgres-admin
Type: "STANDARD"
E2EEKSIAMTestCDP:
Properties:
AssumeRolePolicyDocument: !Sub
- |
{
"Version": "2012-10-17",
"Statement": [
{
"Effect": "Allow",
"Principal": {
"Federated": [
"arn:aws:iam::${AWS::AccountId}:oidc-provider/${OIDC}"
]
},
"Action": [
"sts:AssumeRoleWithWebIdentity"
],
"Condition": {
"StringEquals": {
"${OIDC}:sub": "system:serviceaccount:default:cdp"
}
}
}
]
}
- OIDC: !Select [1, !Split ["//", !GetAtt EKSCluster.OpenIdConnectIssuerUrl]]
Path: /
Policies:
- PolicyDocument:
Statement:
- Action: 'secretsmanager:GetSecretValue'
Effect: Allow
Resource: "arn:aws:secretsmanager:{{.Cluster.Region}}:{{.Cluster.InfrastructureAccountID}}:secret:*.*.*"
RoleName: "{{.Cluster.LocalID}}-cdp"
Type: 'AWS::IAM::Role'
E2EEKSIAMTestDeploymentService:
Properties:
AssumeRolePolicyDocument: !Sub
- |
{
"Version": "2012-10-17",
"Statement": [
{
"Effect": "Allow",
"Principal": {
"Federated": [
"arn:aws:iam::${AWS::AccountId}:oidc-provider/${OIDC}"
]
},
"Action": [
"sts:AssumeRoleWithWebIdentity"
],
"Condition": {
"StringEquals": {
"${OIDC}:sub": "system:serviceaccount:kube-system:deployment-service-controller"
}
}
}
]
}
- OIDC: !Select [1, !Split ["//", !GetAtt EKSCluster.OpenIdConnectIssuerUrl]]
Path: /
Policies:
- PolicyDocument:
Statement:
- Action: 'secretsmanager:GetSecretValue'
Effect: Allow
Resource: "arn:aws:secretsmanager:{{.Cluster.Region}}:{{.Cluster.InfrastructureAccountID}}:secret:*.*.*"
RoleName: "{{.Cluster.LocalID}}-deployment-service"
Type: 'AWS::IAM::Role'
{{ end }}
# TODO: IAM POLICY
EKSCNIIPv6Policy:
Expand Down
139 changes: 134 additions & 5 deletions test/e2e/authorization.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
kubelabels "k8s.io/apimachinery/pkg/labels"
"k8s.io/client-go/kubernetes"
clientset "k8s.io/client-go/kubernetes"
"k8s.io/client-go/rest"
"k8s.io/kubernetes/test/e2e/framework"
testutil "k8s.io/kubernetes/test/utils"
Expand Down Expand Up @@ -497,6 +496,52 @@ var _ = g.Describe("Authorization [RBAC] [Zalando]", func() {
})
})

// Test secret read permissions for CDP and deployment-service
// =============================================================================
// Validates the RBAC permissions granted to CDP and deployment-service for reading
// secrets across all namespaces, including kube-system. These permissions enable
// the workflow where users deploy cluster roles with secret read permissions that
// are subsequently rewritten by the admission controller.
g.When("the service account is deployment-service-controller", func() {
g.BeforeEach(func() {
tc.data.users = []string{"system:serviceaccount:kube-system:deployment-service-controller"}
tc.data.groups = [][]string{{"system:serviceaccounts:kube-system"}}
})
g.It("should allow to read secrets on user namespaces", func() {
tc.data.namespaces = []string{"teapot"}
tc.data.resources = []string{"secrets"}
tc.data.verbs = []string{"read"}
tc.run(context.TODO(), cs, true)
gomega.Expect(tc.output.passed).To(gomega.BeTrue(), tc.output.String())
})
g.It("should allow to read secrets on system namespace", func() {
tc.data.namespaces = []string{"kube-system"}
tc.data.resources = []string{"secrets"}
tc.data.verbs = []string{"read"}
tc.run(context.TODO(), cs, true)
gomega.Expect(tc.output.passed).To(gomega.BeTrue(), tc.output.String())
})
})
g.When("the service account is CDP", func() {
g.BeforeEach(func() {
tc.data.users = []string{"system:serviceaccount:default:cdp"}
tc.data.groups = [][]string{{"system:serviceaccounts:default"}}
})
g.It("should allow to read secrets on user namespaces", func() {
tc.data.namespaces = []string{"teapot"}
tc.data.resources = []string{"secrets"}
tc.data.verbs = []string{"read"}
tc.run(context.TODO(), cs, true)
gomega.Expect(tc.output.passed).To(gomega.BeTrue(), tc.output.String())
})
g.It("should allow to read secrets on system namespace", func() {
tc.data.namespaces = []string{"kube-system"}
tc.data.resources = []string{"secrets"}
tc.data.verbs = []string{"read"}
tc.run(context.TODO(), cs, true)
gomega.Expect(tc.output.passed).To(gomega.BeTrue(), tc.output.String())
})
})
})

g.Context("For administrators", func() {
Expand Down Expand Up @@ -886,6 +931,69 @@ var _ = g.Describe("Authorization via admission-controller [RBAC] [Zalando]", fu
gomega.Expect(result.Error()).To(gomega.MatchError(gomega.ContainSubstring("write operations are forbidden")))
})
})
})

// Test secret read permissions for CDP and deployment-service
// =============================================================================
// Validates that the admission controller correctly rewrites ClusterRole
// permissions related to secret access, ensuring that secret read permissions
// granted to CDP and deployment-service are revoked.
g.Context("cdp and deployment-service", func() {
var (
testSecret *corev1.Secret
systemSecret *corev1.Secret
)

g.BeforeEach(func() {
var err error
testSecret, err = createSecret(context.Background(), f.ClientSet, f.Namespace.Name, map[string]string{"application": "my-app"})
framework.ExpectNoError(err)

systemSecret, err = createSecret(context.Background(), f.ClientSet, "kube-system", map[string]string{"application": "my-app"})
framework.ExpectNoError(err)
})

g.Context("cdp", func() {
var client *kubernetes.Clientset

g.BeforeEach(func() {
var err error

client, err = getCDPClient(eksCluster, awsAccountID)
framework.ExpectNoError(err)
})

g.It("should deny secret read access to user namespace", func() {
_, err := client.CoreV1().Secrets(testSecret.Namespace).Get(context.Background(), testSecret.Name, metav1.GetOptions{})
gomega.Expect(err).To(gomega.MatchError(gomega.ContainSubstring("read operations are forbidden")))
})

g.It("should deny secret read access to kube-system namespace", func() {
_, err := client.CoreV1().Secrets(systemSecret.Namespace).Get(context.Background(), systemSecret.Name, metav1.GetOptions{})
gomega.Expect(err).To(gomega.MatchError(gomega.ContainSubstring("read operations are forbidden")))
})
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, that doesn't add up. First of all, why do we "deny" in both cases? Isn't the point of all this stuff the ability to give permissions on all but one namespace (or some variation of this)? If we want to deny access in all namespaces we could just use RBAC.

Secondly, admission-controller isn't involved in GET requests, so these won't go through admission-controller anyways.

Can we define again what we actually wanted to achieve?

})

g.Context("deployment-service", func() {
var client *kubernetes.Clientset

g.BeforeEach(func() {
var err error

client, err = getDeploymentServiceClient(eksCluster, awsAccountID)
framework.ExpectNoError(err)
})

g.It("should deny secret read access to user namespace", func() {
_, err := client.CoreV1().Secrets(testSecret.Namespace).Get(context.Background(), testSecret.Name, metav1.GetOptions{})
gomega.Expect(err).To(gomega.MatchError(gomega.ContainSubstring("read operations are forbidden")))
})

g.It("should deny secret read access to kube-system namespace", func() {
_, err := client.CoreV1().Secrets(systemSecret.Namespace).Get(context.Background(), systemSecret.Name, metav1.GetOptions{})
gomega.Expect(err).To(gomega.MatchError(gomega.ContainSubstring("read operations are forbidden")))
})
})
})
})

Expand Down Expand Up @@ -914,6 +1022,15 @@ func getPostgresAdministratorClient(cluster *types.Cluster, awsAccountID string)
return newClientWithRole(cluster, fmt.Sprintf("arn:aws:iam::%s:role/%s-e2e-eks-iam-test-postgres-admin-role", awsAccountID, aws.ToString(cluster.Name)))
}

// getCDPClient returns a client with the `zalando:cdp` group.
func getCDPClient(cluster *types.Cluster, awsAccountID string) (*kubernetes.Clientset, error) {
return newClientWithRole(cluster, fmt.Sprintf("arn:aws:iam::%s:role/%s-e2e-eks-iam-test-cdp-role", awsAccountID, aws.ToString(cluster.Name)))
}
// getDeploymentServiceClient returns a client with the `zalando:deployment-service` group.
func getDeploymentServiceClient(cluster *types.Cluster, awsAccountID string) (*kubernetes.Clientset, error) {
return newClientWithRole(cluster, fmt.Sprintf("arn:aws:iam::%s:role/%s-e2e-eks-iam-test-deployment-service-role", awsAccountID, aws.ToString(cluster.Name)))
}

// newClientWithRole returns a new Kubernetes client with the specified IAM role and its associated AccessEntries.
func newClientWithRole(cluster *types.Cluster, assumeRole string) (*kubernetes.Clientset, error) {
gen, err := token.NewGenerator(true, false)
Expand All @@ -932,7 +1049,7 @@ func newClientWithRole(cluster *types.Cluster, assumeRole string) (*kubernetes.C
if err != nil {
return nil, err
}
clientset, err := kubernetes.NewForConfig(
kubernetes, err := kubernetes.NewForConfig(
&rest.Config{
Host: aws.ToString(cluster.Endpoint),
BearerToken: tok.Token,
Expand All @@ -944,7 +1061,7 @@ func newClientWithRole(cluster *types.Cluster, assumeRole string) (*kubernetes.C
if err != nil {
return nil, err
}
return clientset, nil
return kubernetes, nil
}

// getEKSCluster returns the EKS cluster where its Endpoint matches the given config's Host.
Expand Down Expand Up @@ -989,7 +1106,7 @@ func examplePod(namespace string, labels map[string]string) *corev1.Pod {
}

// createPod starts a Pod in the specified namespace and with the specific labels.
func createPod(ctx context.Context, client clientset.Interface, namespace string, labels map[string]string) (*corev1.Pod, error) {
func createPod(ctx context.Context, client kubernetes.Interface, namespace string, labels map[string]string) (*corev1.Pod, error) {
pod, err := client.CoreV1().Pods(namespace).Create(ctx, examplePod(namespace, labels), metav1.CreateOptions{})
if err != nil {
return nil, err
Expand All @@ -1003,7 +1120,7 @@ func createPod(ctx context.Context, client clientset.Interface, namespace string
}

// createClusterRole creates a ClusterRole with the specified labels.
func createClusterRole(ctx context.Context, client clientset.Interface, labels map[string]string) (*rbacv1.ClusterRole, error) {
func createClusterRole(ctx context.Context, client kubernetes.Interface, labels map[string]string) (*rbacv1.ClusterRole, error) {
clusterRole := &rbacv1.ClusterRole{
ObjectMeta: metav1.ObjectMeta{
GenerateName: "test-cluster-role-",
Expand All @@ -1014,6 +1131,18 @@ func createClusterRole(ctx context.Context, client clientset.Interface, labels m
return client.RbacV1().ClusterRoles().Create(ctx, clusterRole, metav1.CreateOptions{})
}

// createSecret creates a Secret with the specified labels.
func createSecret(ctx context.Context, client kubernetes.Interface, namespace string, labels map[string]string) (*corev1.Secret, error) {
secret := &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
GenerateName: "test-secret-",
Labels: labels,
},
}

return client.CoreV1().Secrets(namespace).Create(ctx, secret, metav1.CreateOptions{})
}

// getAWSAccountID returns the current AWS account's ID.
func getAWSAccountID(ctx context.Context, awsConfig aws.Config) (string, error) {
client := sts.NewFromConfig(awsConfig)
Expand Down
File renamed without changes.