fix(tenantresource): ensure original map is not modified in prepareAdditionalMetadata (#1572)

* fix(tenantresource): avoid side effects by copying metadata map instead of returning reference

Signed-off-by: Joshua Leuenberger <joshua.leuenberger@bedag.ch>

* chore(deps): update github/codeql-action digest to 4474150 (#1569)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

chore(tenantresource): make linter happy

Signed-off-by: Joshua Leuenberger <joshua.leuenberger@bedag.ch>

* chore(deps): update all-ci-updates (#1571)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

* chore(deps): update actions/checkout action to v5 (#1577)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

* chore(deps): update dependency pre-commit/pre-commit-hooks to v6 (#1573)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

* chore(deps): update github/codeql-action digest to c6dcdfa (#1575)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

---------

Signed-off-by: Joshua Leuenberger <joshua.leuenberger@bedag.ch>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

.github/workflows/check-pr.yml

@@ -15,7 +15,7 @@ jobs:
     name: Validate PR title
     runs-on: ubuntu-latest
     steps:
-      - uses: amannn/action-semantic-pull-request@fdd4d3ddf614fbcd8c29e4b106d3bbe0cb2c605d
+      - uses: amannn/action-semantic-pull-request@a46a7c8dc4bb34503174eba2f2f7ef80dffc8ed7
         env:
           GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
         with:

.github/workflows/coverage.yml

@@ -52,11 +52,11 @@ jobs:
         with:
           go-version-file: 'go.mod'
       - name: Run Gosec Security Scanner
-        uses: securego/gosec@32975f4bab0d7b683a88756aaf3fa5502188b476 # v2.22.7
+        uses: securego/gosec@c9453023c4e81ebdb6dde29e22d9cd5e2285fb16 # v2.22.8
         with:
           args: '-no-fail -fmt sarif -out gosec.sarif ./...'
       - name: Upload SARIF file
-        uses: github/codeql-action/upload-sarif@7eb43b07886e77d5f743a800ad7a8a380f9f3ea3
+        uses: github/codeql-action/upload-sarif@233052189b8c862bfaf875fb02c115f54d2b9286
         with:
           sarif_file: gosec.sarif
   unit_tests:

.github/workflows/docker-build.yml

@@ -40,6 +40,6 @@ jobs:
           # See: https://github.com/aquasecurity/trivy-action/issues/389#issuecomment-2385416577
           TRIVY_DB_REPOSITORY: 'public.ecr.aws/aquasecurity/trivy-db:2'
       - name: Upload Trivy scan results to GitHub Security tab
-        uses: github/codeql-action/upload-sarif@7eb43b07886e77d5f743a800ad7a8a380f9f3ea3
+        uses: github/codeql-action/upload-sarif@233052189b8c862bfaf875fb02c115f54d2b9286
         with:
           sarif_file: 'trivy-results.sarif'

.github/workflows/releaser.yml

@@ -64,7 +64,7 @@ jobs:
         timeout-minutes: 5
         continue-on-error: true
       - uses: creekorful/goreportcard-action@1f35ced8cdac2cba28c9a2f2288a16aacfd507f9 # v1.0
-      - uses: anchore/sbom-action/download-syft@7b36ad622f042cab6f59a75c2ac24ccb256e9b45
+      - uses: anchore/sbom-action/download-syft@da167eac915b4e86f08b264dbdbc867b61be6f0c
       - name: Install Cosign
         uses: sigstore/cosign-installer@d58896d6a1865668819e1d91763c7751a165e159 # v3.9.2
       - name: download artifact
@@ -73,7 +73,7 @@ jobs:
           name: capsule-seccomp
           path: ./capsule-seccomp.json
       - name: Run GoReleaser
-        uses: goreleaser/goreleaser-action@9c156ee8a17a598857849441385a2041ef570552 # v6.3.0
+        uses: goreleaser/goreleaser-action@e435ccd777264be153ace6237001ef4d979d3a7a # v6.4.0
         with:
           version: latest
           args: release --clean --timeout 90m

.golangci.yaml

@@ -23,6 +23,7 @@ linters:
     - unparam
     - varnamelen
     - wrapcheck
+    - interfacebloat
     - noinlineerr
     - revive
   settings:

api/v1beta2/capsuleconfiguration_types.go

@@ -14,6 +14,9 @@ type CapsuleConfigurationSpec struct {
 	// Names of the groups for Capsule users.
 	// +kubebuilder:default={capsule.clastix.io}
 	UserGroups []string `json:"userGroups,omitempty"`
+	// Define groups which when found in the request of a user will be ignored by the Capsule
+	// this might be useful if you have one group where all the users are in, but you want to separate administrators from normal users with additional groups.
+	IgnoreUserWithGroups []string `json:"ignoreUserWithGroups,omitempty"`
 	// Enforces the Tenant owner, during Namespace creation, to name it using the selected Tenant name as prefix,
 	// separated by a dash. This is useful to avoid Namespace name collision in a public CaaS environment.
 	// +kubebuilder:default=false

api/v1beta2/zz_generated.deepcopy.go

@@ -122,6 +122,11 @@ func (in *CapsuleConfigurationSpec) DeepCopyInto(out *CapsuleConfigurationSpec)
 		*out = make([]string, len(*in))
 		copy(*out, *in)
 	}
+	if in.IgnoreUserWithGroups != nil {
+		in, out := &in.IgnoreUserWithGroups, &out.IgnoreUserWithGroups
+		*out = make([]string, len(*in))
+		copy(*out, *in)
+	}
 	out.CapsuleResources = in.CapsuleResources
 	if in.NodeMetadata != nil {
 		in, out := &in.NodeMetadata, &out.NodeMetadata

charts/capsule/crds/capsule.clastix.io_capsuleconfigurations.yaml

@@ -52,6 +52,13 @@ spec:
                   Enforces the Tenant owner, during Namespace creation, to name it using the selected Tenant name as prefix,
                   separated by a dash. This is useful to avoid Namespace name collision in a public CaaS environment.
                 type: boolean
+              ignoreUserWithGroups:
+                description: |-
+                  Define groups which when found in the request of a user will be ignored by the Capsule
+                  this might be useful if you have one group where all the users are in, but you want to separate administrators from normal users with additional groups.
+                items:
+                  type: string
+                type: array
               nodeMetadata:
                 description: |-
                   Allows to set the forbidden metadata for the worker nodes that could be patched by a Tenant.

controllers/resources/processor.go

@@ -40,7 +40,13 @@ func prepareAdditionalMetadata(m map[string]string) map[string]string {
 		return make(map[string]string)
 	}
 
-	return m
+	// we need to create a new map to avoid modifying the original one
+	copied := make(map[string]string, len(m))
+	for k, v := range m {
+		copied[k] = v
+	}
+
+	return copied
 }
 
 func (r *Processor) HandlePruning(ctx context.Context, current, desired sets.Set[string]) (updateStatus bool) {

e2e/custom_capsule_group_test.go

@@ -9,11 +9,14 @@ import (
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	capsulev1beta2 "github.com/projectcapsule/capsule/api/v1beta2"
 )
 
 var _ = Describe("creating a Namespace as Tenant owner with custom --capsule-group", Label("config"), func() {
+	originConfig := &capsulev1beta2.CapsuleConfiguration{}
+
 	tnt := &capsulev1beta2.Tenant{
 		ObjectMeta: metav1.ObjectMeta{
 			Name: "tenant-assigned-custom-group",
@@ -29,6 +32,8 @@ var _ = Describe("creating a Namespace as Tenant owner with custom --capsule-gro
 	}
 
 	JustBeforeEach(func() {
+		Expect(k8sClient.Get(context.Background(), client.ObjectKey{Name: defaultConfigurationName}, originConfig)).To(Succeed())
+
 		EventuallyCreation(func() error {
 			tnt.ResourceVersion = ""
 			return k8sClient.Create(context.TODO(), tnt)
@@ -36,6 +41,17 @@ var _ = Describe("creating a Namespace as Tenant owner with custom --capsule-gro
 	})
 	JustAfterEach(func() {
 		Expect(k8sClient.Delete(context.TODO(), tnt)).Should(Succeed())
+
+		// Restore Configuration
+		Eventually(func() error {
+			c := &capsulev1beta2.CapsuleConfiguration{}
+			if err := k8sClient.Get(context.Background(), client.ObjectKey{Name: originConfig.Name}, c); err != nil {
+				return err
+			}
+			// Apply the initial configuration from originConfig to c
+			c.Spec = originConfig.Spec
+			return k8sClient.Update(context.Background(), c)
+		}, defaultTimeoutInterval, defaultPollInterval).Should(Succeed())
 	})
 
 	It("should fail using a User non matching the capsule-user-group flag", func() {
@@ -68,4 +84,16 @@ var _ = Describe("creating a Namespace as Tenant owner with custom --capsule-gro
 		NamespaceCreation(ns, tnt.Spec.Owners[0], defaultTimeoutInterval).Should(Succeed())
 		TenantNamespaceList(tnt, defaultTimeoutInterval).Should(ContainElement(ns.GetName()))
 	})
+
+	It("should fail when group is ignored", func() {
+		ModifyCapsuleConfigurationOpts(func(configuration *capsulev1beta2.CapsuleConfiguration) {
+			configuration.Spec.UserGroups = []string{"projectcapsule.dev"}
+			configuration.Spec.IgnoreUserWithGroups = []string{"projectcapsule.dev"}
+		})
+
+		ns := NewNamespace("")
+
+		NamespaceCreation(ns, tnt.Spec.Owners[0], defaultTimeoutInterval).ShouldNot(Succeed())
+	})
+
 })

e2e/node_user_metadata_test.go

@@ -12,6 +12,7 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	rbacv1 "k8s.io/api/rbac/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	capsulev1beta2 "github.com/projectcapsule/capsule/api/v1beta2"
 	"github.com/projectcapsule/capsule/pkg/api"
@@ -19,6 +20,8 @@ import (
 )
 
 var _ = Describe("modifying node labels and annotations", Label("config", "nodes"), func() {
+	originConfig := &capsulev1beta2.CapsuleConfiguration{}
+
 	tnt := &capsulev1beta2.Tenant{
 		ObjectMeta: metav1.ObjectMeta{
 			Name: "tenant-node-user-metadata-forbidden",
@@ -72,6 +75,8 @@ var _ = Describe("modifying node labels and annotations", Label("config", "nodes
 			Skip(fmt.Sprintf("Node webhook is disabled for current version %s", version.String()))
 		}
 
+		Expect(k8sClient.Get(context.Background(), client.ObjectKey{Name: defaultConfigurationName}, originConfig)).To(Succeed())
+
 		EventuallyCreation(func() error {
 			tnt.ResourceVersion = ""
 			return k8sClient.Create(context.TODO(), tnt)
@@ -110,6 +115,17 @@ var _ = Describe("modifying node labels and annotations", Label("config", "nodes
 				return k8sClient.Update(context.Background(), node)
 			})
 		}).Should(Succeed())
+
+		// Restore Configuration
+		Eventually(func() error {
+			c := &capsulev1beta2.CapsuleConfiguration{}
+			if err := k8sClient.Get(context.Background(), client.ObjectKey{Name: originConfig.Name}, c); err != nil {
+				return err
+			}
+			// Apply the initial configuration from originConfig to c
+			c.Spec = originConfig.Spec
+			return k8sClient.Update(context.Background(), c)
+		}, defaultTimeoutInterval, defaultPollInterval).Should(Succeed())
 	})
 
 	It("should allow", func() {
@@ -171,6 +187,19 @@ var _ = Describe("modifying node labels and annotations", Label("config", "nodes
 		})
 	})
 	It("should fail", func() {
+		ModifyCapsuleConfigurationOpts(func(configuration *capsulev1beta2.CapsuleConfiguration) {
+			configuration.Spec.NodeMetadata = &capsulev1beta2.NodeMetadata{
+				ForbiddenLabels: api.ForbiddenListSpec{
+					Exact: []string{"foo", "bar"},
+					Regex: "^gatsby-.*$",
+				},
+				ForbiddenAnnotations: api.ForbiddenListSpec{
+					Exact: []string{"foo", "bar"},
+					Regex: "^gatsby-.*$",
+				},
+			}
+		})
+
 		Expect(ModifyNode(func(node *corev1.Node) error {
 			node.Labels["foo"] = "bar"
 			node.Labels["gatsby-foo"] = "bar"

e2e/protected_namespace_regex_test.go

@@ -9,11 +9,14 @@ import (
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	capsulev1beta2 "github.com/projectcapsule/capsule/api/v1beta2"
 )
 
 var _ = Describe("creating a Namespace with a protected Namespace regex enabled", Label("namespace"), func() {
+	originConfig := &capsulev1beta2.CapsuleConfiguration{}
+
 	tnt := &capsulev1beta2.Tenant{
 		ObjectMeta: metav1.ObjectMeta{
 			Name: "tenant-protected-namespace",
@@ -29,6 +32,8 @@ var _ = Describe("creating a Namespace with a protected Namespace regex enabled"
 	}
 
 	JustBeforeEach(func() {
+		Expect(k8sClient.Get(context.Background(), client.ObjectKey{Name: defaultConfigurationName}, originConfig)).To(Succeed())
+
 		EventuallyCreation(func() error {
 			tnt.ResourceVersion = ""
 			return k8sClient.Create(context.TODO(), tnt)
@@ -36,6 +41,17 @@ var _ = Describe("creating a Namespace with a protected Namespace regex enabled"
 	})
 	JustAfterEach(func() {
 		Expect(k8sClient.Delete(context.TODO(), tnt)).Should(Succeed())
+
+		// Restore Configuration
+		Eventually(func() error {
+			c := &capsulev1beta2.CapsuleConfiguration{}
+			if err := k8sClient.Get(context.Background(), client.ObjectKey{Name: originConfig.Name}, c); err != nil {
+				return err
+			}
+			// Apply the initial configuration from originConfig to c
+			c.Spec = originConfig.Spec
+			return k8sClient.Update(context.Background(), c)
+		}, defaultTimeoutInterval, defaultPollInterval).Should(Succeed())
 	})
 
 	It("should succeed and be available in Tenant namespaces list", func() {

e2e/utils_test.go

@@ -27,8 +27,9 @@ import (
 )
 
 const (
-	defaultTimeoutInterval = 40 * time.Second
-	defaultPollInterval    = time.Second
+	defaultTimeoutInterval   = 40 * time.Second
+	defaultPollInterval      = time.Second
+	defaultConfigurationName = "default"
 )
 
 func NewService(svc types.NamespacedName) *corev1.Service {
@@ -100,7 +101,7 @@ func EventuallyCreation(f interface{}) AsyncAssertion {
 
 func ModifyCapsuleConfigurationOpts(fn func(configuration *capsulev1beta2.CapsuleConfiguration)) {
 	config := &capsulev1beta2.CapsuleConfiguration{}
-	Expect(k8sClient.Get(context.Background(), types.NamespacedName{Name: "default"}, config)).ToNot(HaveOccurred())
+	Expect(k8sClient.Get(context.Background(), types.NamespacedName{Name: defaultConfigurationName}, config)).ToNot(HaveOccurred())
 
 	fn(config)
 

pkg/configuration/client.go

@@ -85,6 +85,10 @@ func (c *capsuleConfiguration) UserGroups() []string {
 	return c.retrievalFn().Spec.UserGroups
 }
 
+func (c *capsuleConfiguration) IgnoreUserWithGroups() []string {
+	return c.retrievalFn().Spec.IgnoreUserWithGroups
+}
+
 func (c *capsuleConfiguration) ForbiddenUserNodeLabels() *capsuleapi.ForbiddenListSpec {
 	if c.retrievalFn().Spec.NodeMetadata == nil {
 		return nil

pkg/configuration/configuration.go

@@ -24,6 +24,7 @@ type Configuration interface {
 	ValidatingWebhookConfigurationName() string
 	TenantCRDName() string
 	UserGroups() []string
+	IgnoreUserWithGroups() []string
 	ForbiddenUserNodeLabels() *capsuleapi.ForbiddenListSpec
 	ForbiddenUserNodeAnnotations() *capsuleapi.ForbiddenListSpec
 }

pkg/webhook/namespace/validation/freezed.go

@@ -74,7 +74,7 @@ func (r *freezedHandler) OnDelete(c client.Client, _ admission.Decoder, recorder
 
 		tnt := tntList.Items[0]
 
-		if tnt.Spec.Cordoned && utils.IsCapsuleUser(ctx, req, c, r.configuration.UserGroups()) {
+		if tnt.Spec.Cordoned && utils.IsCapsuleUser(ctx, req, c, r.configuration.UserGroups(), r.configuration.IgnoreUserWithGroups()) {
 			recorder.Eventf(&tnt, corev1.EventTypeWarning, "TenantFreezed", "Namespace %s cannot be deleted, the current Tenant is freezed", req.Name)
 
 			response := admission.Denied("the selected Tenant is freezed")
@@ -106,7 +106,7 @@ func (r *freezedHandler) OnUpdate(c client.Client, decoder admission.Decoder, re
 
 		tnt := tntList.Items[0]
 
-		if tnt.Spec.Cordoned && utils.IsCapsuleUser(ctx, req, c, r.configuration.UserGroups()) {
+		if tnt.Spec.Cordoned && utils.IsCapsuleUser(ctx, req, c, r.configuration.UserGroups(), r.configuration.IgnoreUserWithGroups()) {
 			recorder.Eventf(&tnt, corev1.EventTypeWarning, "TenantFreezed", "Namespace %s cannot be updated, the current Tenant is freezed", ns.GetName())
 
 			response := admission.Denied("the selected Tenant is freezed")

pkg/webhook/tenant/cordoning.go

@@ -62,7 +62,7 @@ func (h *cordoningHandler) cordonHandler(ctx context.Context, clt client.Client,
 	}
 
 	tnt := tntList.Items[0]
-	if tnt.Spec.Cordoned && utils.IsCapsuleUser(ctx, req, clt, h.configuration.UserGroups()) {
+	if tnt.Spec.Cordoned && utils.IsCapsuleUser(ctx, req, clt, h.configuration.UserGroups(), h.configuration.IgnoreUserWithGroups()) {
 		recorder.Eventf(&tnt, corev1.EventTypeWarning, "TenantFreezed", "%s %s/%s cannot be %sd, current Tenant is freezed", req.Kind.String(), req.Namespace, req.Name, strings.ToLower(string(req.Operation)))
 
 		response := admission.Denied(fmt.Sprintf("tenant %s is freezed: please, reach out to the system administrator", tnt.GetName()))

pkg/webhook/utils/in_capsule_groups.go

@@ -26,9 +26,10 @@ type handler struct {
 	handlers      []webhook.Handler
 }
 
+//nolint:dupl
 func (h *handler) OnCreate(client client.Client, decoder admission.Decoder, recorder record.EventRecorder) webhook.Func {
 	return func(ctx context.Context, req admission.Request) *admission.Response {
-		if !IsCapsuleUser(ctx, req, client, h.configuration.UserGroups()) {
+		if !IsCapsuleUser(ctx, req, client, h.configuration.UserGroups(), h.configuration.IgnoreUserWithGroups()) {
 			return nil
 		}
 
@@ -42,9 +43,10 @@ func (h *handler) OnCreate(client client.Client, decoder admission.Decoder, reco
 	}
 }
 
+//nolint:dupl
 func (h *handler) OnDelete(client client.Client, decoder admission.Decoder, recorder record.EventRecorder) webhook.Func {
 	return func(ctx context.Context, req admission.Request) *admission.Response {
-		if !IsCapsuleUser(ctx, req, client, h.configuration.UserGroups()) {
+		if !IsCapsuleUser(ctx, req, client, h.configuration.UserGroups(), h.configuration.IgnoreUserWithGroups()) {
 			return nil
 		}
 
@@ -58,9 +60,10 @@ func (h *handler) OnDelete(client client.Client, decoder admission.Decoder, reco
 	}
 }
 
+//nolint:dupl
 func (h *handler) OnUpdate(client client.Client, decoder admission.Decoder, recorder record.EventRecorder) webhook.Func {
 	return func(ctx context.Context, req admission.Request) *admission.Response {
-		if !IsCapsuleUser(ctx, req, client, h.configuration.UserGroups()) {
+		if !IsCapsuleUser(ctx, req, client, h.configuration.UserGroups(), h.configuration.IgnoreUserWithGroups()) {
 			return nil
 		}
 

pkg/webhook/utils/is_capsule_user.go

@@ -16,7 +16,7 @@ import (
 	"github.com/projectcapsule/capsule/pkg/utils"
 )
 
-func IsCapsuleUser(ctx context.Context, req admission.Request, clt client.Client, userGroups []string) bool {
+func IsCapsuleUser(ctx context.Context, req admission.Request, clt client.Client, userGroups []string, ignoreGroups []string) bool {
 	groupList := utils.NewUserGroupList(req.UserInfo.Groups)
 	// if the user is a ServiceAccount belonging to the kube-system namespace, definitely, it's not a Capsule user
 	// and we can skip the check in case of Capsule user group assigned to system:authenticated
@@ -44,6 +44,14 @@ func IsCapsuleUser(ctx context.Context, req admission.Request, clt client.Client
 
 	for _, group := range userGroups {
 		if groupList.Find(group) {
+			if len(ignoreGroups) > 0 {
+				for _, ignoreGroup := range ignoreGroups {
+					if groupList.Find(ignoreGroup) {
+						return false
+					}
+				}
+			}
+
 			return true
 		}
 	}