From 12f54f680dc09c572bb4aa183d68eadabd094bd2 Mon Sep 17 00:00:00 2001 From: Yash Sethiya Date: Tue, 5 Mar 2024 15:32:30 +0530 Subject: [PATCH 1/2] Default rule to make sure secret associated with service-account created after service-account is created Signed-off-by: Yash Sethiya --- pkg/kapp/config/default.go | 14 +++++ test/e2e/change_rule_test.go | 119 +++++++++++++++++++++++++++++++++++ 2 files changed, 133 insertions(+) create mode 100644 test/e2e/change_rule_test.go diff --git a/pkg/kapp/config/default.go b/pkg/kapp/config/default.go index cf4a772de..9eb8625fa 100644 --- a/pkg/kapp/config/default.go +++ b/pkg/kapp/config/default.go @@ -569,6 +569,14 @@ changeGroupBindings: resourceMatchers: &serviceAccountMatchers - apiVersionKindMatcher: {kind: ServiceAccount, apiVersion: v1} +- name: change-groups.kapp.k14s.io/secret-associated-with-sa + resourceMatchers: + - andMatcher: + matchers: + - apiVersionKindMatcher: {kind: Secret, apiVersion: v1} + - hasAnnotationMatcher: + keys: [kubernetes.io/service-account.name] + - name: change-groups.kapp.k14s.io/kapp-controller-app resourceMatchers: *appMatchers @@ -588,6 +596,12 @@ changeRuleBindings: hasAnnotationMatcher: keys: [kapp.k14s.io/disable-default-change-group-and-rules] +# Create SA before creating secret associated with SA +- rules: + - "upsert before upserting change-groups.kapp.k14s.io/secret-associated-with-sa" + resourceMatchers: + - apiVersionKindMatcher: {kind: ServiceAccount, apiVersion: v1} + # Delete CRs before CRDs to retain detailed observability # instead of having CRD deletion trigger all CR deletion - rules: diff --git a/test/e2e/change_rule_test.go b/test/e2e/change_rule_test.go new file mode 100644 index 000000000..97c87bc37 --- /dev/null +++ b/test/e2e/change_rule_test.go @@ -0,0 +1,119 @@ +// Copyright 2020 VMware, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package e2e + +import ( + "strings" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestServiceAccountAssociatedSecretDefaultChangeRule(t *testing.T) { + env := BuildEnv(t) + logger := Logger{} + kapp := Kapp{t, env.Namespace, env.KappBinaryPath, logger} + + config := ` +apiVersion: v1 +kind: Namespace +metadata: + name: kapp-token-test +--- +apiVersion: v1 +kind: ServiceAccount +metadata: + name: sa-0 + namespace: kapp-token-test +--- +apiVersion: v1 +kind: Secret +metadata: + name: secret-0 + namespace: kapp-token-test + annotations: + kubernetes.io/service-account.name: sa-0 +type: kubernetes.io/service-account-token +--- +apiVersion: v1 +kind: ServiceAccount +metadata: + name: sa-1 + namespace: kapp-token-test +--- +apiVersion: v1 +kind: Secret +metadata: + name: secret-1 + namespace: kapp-token-test + annotations: + kubernetes.io/service-account.name: sa-1 +type: kubernetes.io/service-account-token +--- +apiVersion: v1 +kind: ServiceAccount +metadata: + name: sa-2 + namespace: kapp-token-test +--- +apiVersion: v1 +kind: Secret +metadata: + name: secret-2 + namespace: kapp-token-test + annotations: + kubernetes.io/service-account.name: sa-2 +type: kubernetes.io/service-account-token +--- +apiVersion: v1 +kind: ServiceAccount +metadata: + name: sa-3 + namespace: kapp-token-test +--- +apiVersion: v1 +kind: Secret +metadata: + name: secret-3 + namespace: kapp-token-test + annotations: + kubernetes.io/service-account.name: sa-3 +type: kubernetes.io/service-account-token +--- +apiVersion: v1 +kind: ServiceAccount +metadata: + name: sa-4 + namespace: kapp-token-test +--- +apiVersion: v1 +kind: Secret +metadata: + name: secret-4 + namespace: kapp-token-test + annotations: + kubernetes.io/service-account.name: sa-4 +type: kubernetes.io/service-account-token +` + + name := "test-sa-created-before-secret-associated-with-sa" + cleanUp := func() { + kapp.Run([]string{"delete", "-a", name}) + } + + cleanUp() + + logger.Section("deploy resource with secret associated with service account", func() { + // deploying it multiple times to make sure it's able to find secret everytime + for i := 0; i < 5; i++ { + _, err := kapp.RunWithOpts([]string{"deploy", "-f", "-", "-a", name}, RunOpts{ + StdinReader: strings.NewReader(config), + }) + if err != nil { + require.Errorf(t, err, "Expected resources to be created") + } + cleanUp() + } + }) +} From 128333184dc864c34dbeb700415cd13246de8675 Mon Sep 17 00:00:00 2001 From: Yash Sethiya Date: Tue, 12 Mar 2024 21:52:45 +0530 Subject: [PATCH 2/2] Added unit test Signed-off-by: Yash Sethiya --- pkg/kapp/diffgraph/change_graph_test.go | 27 +++++++++++++++++++++++++ test/e2e/change_rule_test.go | 2 +- 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/pkg/kapp/diffgraph/change_graph_test.go b/pkg/kapp/diffgraph/change_graph_test.go index ca0eb499a..b1d0c763a 100644 --- a/pkg/kapp/diffgraph/change_graph_test.go +++ b/pkg/kapp/diffgraph/change_graph_test.go @@ -809,6 +809,33 @@ metadata: require.Equal(t, expectedOutput, output) } +func TestChangeGraphWithSecretAndSA(t *testing.T) { + yaml := ` +apiVersion: v1 +kind: ServiceAccount +metadata: + name: sa-0 +--- +apiVersion: v1 +kind: Secret +metadata: + name: secret-0 + annotations: + kubernetes.io/service-account.name: sa-0 +type: kubernetes.io/service-account-token +` + + graph, err := buildChangeGraph(yaml, ctldgraph.ActualChangeOpUpsert, t) + require.NoErrorf(t, err, "Expected graph to build") + + output := strings.TrimSpace(graph.PrintStr()) + expectedOutput := strings.TrimSpace(` +(upsert) serviceaccount/sa-0 (v1) cluster +(upsert) secret/secret-0 (v1) cluster +`) + require.Equal(t, expectedOutput, output) +} + func buildChangeGraph(resourcesBs string, op ctldgraph.ActualChangeOp, t *testing.T) (*ctldgraph.ChangeGraph, error) { return buildChangeGraphWithOpts(buildGraphOpts{resourcesBs: resourcesBs, op: op}, t) } diff --git a/test/e2e/change_rule_test.go b/test/e2e/change_rule_test.go index 97c87bc37..d3b9be649 100644 --- a/test/e2e/change_rule_test.go +++ b/test/e2e/change_rule_test.go @@ -1,4 +1,4 @@ -// Copyright 2020 VMware, Inc. +// Copyright 2024 The Carvel Authors. // SPDX-License-Identifier: Apache-2.0 package e2e