From fc4fb6107692e9e6e1ade4398368038a2f9a3da3 Mon Sep 17 00:00:00 2001 From: Johannes Aubart Date: Tue, 20 May 2025 09:17:43 +0200 Subject: [PATCH 1/3] add clusteraccess library --- docs/README.md | 1 + docs/libs/clusteraccess.md | 3 + pkg/clusteraccess/access.go | 336 ++++++++++++++++++++++++++ pkg/clusteraccess/access_test.go | 390 +++++++++++++++++++++++++++++++ pkg/clusteraccess/errors.go | 71 ++++++ pkg/clusteraccess/labels.go | 55 +++++ pkg/clusteraccess/suite_test.go | 14 ++ 7 files changed, 870 insertions(+) create mode 100644 docs/libs/clusteraccess.md create mode 100644 pkg/clusteraccess/access.go create mode 100644 pkg/clusteraccess/access_test.go create mode 100644 pkg/clusteraccess/errors.go create mode 100644 pkg/clusteraccess/labels.go create mode 100644 pkg/clusteraccess/suite_test.go diff --git a/docs/README.md b/docs/README.md index e51bc7b..a9b801b 100644 --- a/docs/README.md +++ b/docs/README.md @@ -3,6 +3,7 @@ ## Libraries +- [Generating Kubeconfigs for k8s Clusters](libs/clusteraccess.md) - [Connecting to Kubernetes Clusters](libs/clusters.md) - [Collections](libs/collections.md) - [Controller Utility Functions](libs/controller.md) diff --git a/docs/libs/clusteraccess.md b/docs/libs/clusteraccess.md new file mode 100644 index 0000000..f46024a --- /dev/null +++ b/docs/libs/clusteraccess.md @@ -0,0 +1,3 @@ +# Generating Kubeconfigs for k8s Clusters + +The `pkg/clusteraccess` package contains useful helper functions to create a kubeconfig for a k8s cluster. This includes functions to create ServiceAccounts as well as (Cluster)Roles and (Cluster)RoleBindings, but also generating a ServiceAccount token and building a kubeconfig from this token. \ No newline at end of file diff --git a/pkg/clusteraccess/access.go b/pkg/clusteraccess/access.go new file mode 100644 index 0000000..4a8c4dc --- /dev/null +++ b/pkg/clusteraccess/access.go @@ -0,0 +1,336 @@ +package clusteraccess + +import ( + "context" + "fmt" + "time" + + authenticationv1 "k8s.io/api/authentication/v1" + corev1 "k8s.io/api/core/v1" + rbacv1 "k8s.io/api/rbac/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/client-go/rest" + "k8s.io/client-go/tools/clientcmd" + clientcmdapi "k8s.io/client-go/tools/clientcmd/api" + "k8s.io/utils/ptr" + "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/openmcp-project/controller-utils/pkg/resources" +) + +// GetTokenBasedAccess is a convenience function that wraps the flow of ensuring namespace, serviceaccount, (cluster)role(binding), and creating the token. +// It returns a kubeconfig, the token with expiration timestamp, and an error if any of the steps fail. +// The name will be used for all resources except the namespace (serviceaccount, (cluster)role, (cluster)rolebinding), with anything role-related additionally being prefixed with rolePrefix. +// The namespace holds the serviceaccount and, if namespaceScoped is true, the role and rolebinding. +// If namespaceScoped is false, clusterrole and clusterrolebinding are used. +func GetTokenBasedAccess(ctx context.Context, c client.Client, restCfg *rest.Config, name, namespace string, namespaceScoped bool, rolePrefix string, rules []rbacv1.PolicyRule, expectedLabels ...Label) ([]byte, *ServiceAccountToken, error) { + if namespace == "" { + return nil, nil, fmt.Errorf("no namespace provided for ServiceAccount") + } + + _, err := EnsureNamespace(ctx, c, namespace, expectedLabels...) + if err != nil { + return nil, nil, err + } + + sa, err := EnsureServiceAccount(ctx, c, name, namespace, expectedLabels...) + if err != nil { + return nil, nil, err + } + + subjects := []rbacv1.Subject{{Kind: rbacv1.ServiceAccountKind, Name: name, Namespace: namespace}} + if namespaceScoped { + _, _, err = EnsureRoleAndBinding(ctx, c, rolePrefix+name, namespace, subjects, rules, expectedLabels...) + if err != nil { + return nil, nil, err + } + } else { + _, _, err = EnsureClusterRoleAndBinding(ctx, c, rolePrefix+name, subjects, rules, expectedLabels...) + if err != nil { + return nil, nil, err + } + } + + sat, err := CreateTokenForServiceAccount(ctx, c, sa, nil) + if err != nil { + return nil, nil, err + } + + kcfg, err := CreateTokenKubeconfig(name, restCfg.Host, restCfg.CAData, sat.Token) + if err != nil { + return nil, nil, err + } + + return kcfg, sat, nil +} + +// EnsureNamespace ensures that the specified Namespace exists. +// If it doesn't exist, it is created with the expected labels. +// If it exists, but does not have the expected labels, a ResourceNotManagedError is returned. +// The namespace is returned. +func EnsureNamespace(ctx context.Context, c client.Client, nsName string, expectedLabels ...Label) (*corev1.Namespace, error) { + ns := &corev1.Namespace{} + ns.SetName(nsName) + found := true + if err := c.Get(ctx, client.ObjectKeyFromObject(ns), ns); err != nil { + if !apierrors.IsNotFound(err) { + return nil, fmt.Errorf("error getting Namespace '%s': %w", ns.Name, err) + } + found = false + } + if found { + if err := FailIfNotManaged(ns, expectedLabels...); err != nil { + return nil, err + } + // a namespace does not have any spec, so we don't have to do anything, if it was found + return ns, nil + } + ns.SetLabels(LabelListToMap(expectedLabels)) + if err := c.Create(ctx, ns); err != nil { + return nil, fmt.Errorf("error creating Namespace '%s': %w", ns.Name, err) + } + + return ns, nil +} + +// EnsureServiceAccount ensures that the specified ServiceAccount exists. +// If it doesn't exist, it is created with the expected labels (the namespace has to exist). +// If it exists, but does not have the expected labels, a ResourceNotManagedError is returned. +// The ServiceAccount is returned. +func EnsureServiceAccount(ctx context.Context, c client.Client, saName, saNamespace string, expectedLabels ...Label) (*corev1.ServiceAccount, error) { + sa := &corev1.ServiceAccount{} + sa.SetName(saName) + sa.SetNamespace(saNamespace) + found := true + if err := c.Get(ctx, client.ObjectKeyFromObject(sa), sa); err != nil { + if !apierrors.IsNotFound(err) { + return nil, fmt.Errorf("error getting ServiceAccount '%s/%s': %w", sa.Namespace, sa.Name, err) + } + found = false + } + if found { + if err := FailIfNotManaged(sa, expectedLabels...); err != nil { + return nil, err + } + // a serviceaccount does not have any relevant spec, so we don't have to do anything, if it was found + return sa, nil + } + sa.SetLabels(LabelListToMap(expectedLabels)) + if err := c.Create(ctx, sa); err != nil { + return nil, fmt.Errorf("error creating ServiceAccount '%s': %w", sa.Name, err) + } + + return sa, nil +} + +// EnsureClusterRoleAndBinding combines EnsureClusterRole and EnsureClusterRoleBinding. +// The name is used for both the ClusterRole and ClusterRoleBinding. +func EnsureClusterRoleAndBinding(ctx context.Context, c client.Client, name string, subjects []rbacv1.Subject, rules []rbacv1.PolicyRule, expectedLabels ...Label) (*rbacv1.ClusterRoleBinding, *rbacv1.ClusterRole, error) { + cr, err := EnsureClusterRole(ctx, c, name, rules, expectedLabels...) + if err != nil { + return nil, nil, err + } + crb, err := EnsureClusterRoleBinding(ctx, c, name, cr.Name, subjects, expectedLabels...) + if err != nil { + return nil, cr, err + } + return crb, cr, nil +} + +// EnsureClusterRole ensures that the specified ClusterRole exists with the specified rules. +// If it doesn't exist, it is created with the expected labels. +// If it exists, but does not have the expected labels, a ResourceNotManagedError is returned. +// The ClusterRole is returned. +func EnsureClusterRole(ctx context.Context, c client.Client, name string, rules []rbacv1.PolicyRule, expectedLabels ...Label) (*rbacv1.ClusterRole, error) { + crm := resources.NewClusterRoleMutator(name, rules, LabelListToMap(expectedLabels), nil) + cr := crm.Empty() + found := true + if err := c.Get(ctx, client.ObjectKeyFromObject(cr), cr); err != nil { + if !apierrors.IsNotFound(err) { + return nil, fmt.Errorf("error getting ClusterRole '%s': %w", cr.Name, err) + } + found = false + } + if found { + if err := FailIfNotManaged(cr, expectedLabels...); err != nil { + return nil, err + } + } + if err := resources.CreateOrUpdateResource(ctx, c, crm); err != nil { + return nil, fmt.Errorf("error creating/updating ClusterRole '%s': %w", cr.Name, err) + } + return cr, nil +} + +// EnsureClusterRoleBinding ensures that the specified ClusterRoleBinding exists with the specified subjects. +// If it doesn't exist, it is created with the expected labels. +// If it exists, but does not have the expected labels, a ResourceNotManagedError is returned. +// The ClusterRoleBinding is returned. +func EnsureClusterRoleBinding(ctx context.Context, c client.Client, name, clusterRoleName string, subjects []rbacv1.Subject, expectedLabels ...Label) (*rbacv1.ClusterRoleBinding, error) { + crbm := resources.NewClusterRoleBindingMutator(name, subjects, resources.NewClusterRoleRef(clusterRoleName), LabelListToMap(expectedLabels), nil) + crb := crbm.Empty() + found := true + if err := c.Get(ctx, client.ObjectKeyFromObject(crb), crb); err != nil { + if !apierrors.IsNotFound(err) { + return nil, fmt.Errorf("error getting ClusterRoleBinding '%s': %w", crb.Name, err) + } + found = false + } + if found { + if err := FailIfNotManaged(crb, expectedLabels...); err != nil { + return nil, err + } + } + if err := resources.CreateOrUpdateResource(ctx, c, crbm); err != nil { + return nil, fmt.Errorf("error creating/updating ClusterRole '%s': %w", crb.Name, err) + } + return crb, nil +} + +// EnsureRoleAndBinding combines EnsureRole and EnsureRoleBinding. +// The name is used for both the Role and RoleBinding. +func EnsureRoleAndBinding(ctx context.Context, c client.Client, name, namespace string, subjects []rbacv1.Subject, rules []rbacv1.PolicyRule, expectedLabels ...Label) (*rbacv1.RoleBinding, *rbacv1.Role, error) { + r, err := EnsureRole(ctx, c, name, namespace, rules, expectedLabels...) + if err != nil { + return nil, nil, err + } + rb, err := EnsureRoleBinding(ctx, c, name, namespace, r.Name, subjects, expectedLabels...) + if err != nil { + return nil, r, err + } + return rb, r, nil +} + +// EnsureRole ensures that the specified Role exists with the specified rules. +// If it doesn't exist, it is created with the expected labels. +// If it exists, but does not have the expected labels, a ResourceNotManagedError is returned. +// The Role is returned. +func EnsureRole(ctx context.Context, c client.Client, name, namespace string, rules []rbacv1.PolicyRule, expectedLabels ...Label) (*rbacv1.Role, error) { + rm := resources.NewRoleMutator(name, namespace, rules, LabelListToMap(expectedLabels), nil) + r := rm.Empty() + found := true + if err := c.Get(ctx, client.ObjectKeyFromObject(r), r); err != nil { + if !apierrors.IsNotFound(err) { + return nil, fmt.Errorf("error getting Role '%s/%s': %w", r.Namespace, r.Name, err) + } + found = false + } + if found { + if err := FailIfNotManaged(r, expectedLabels...); err != nil { + return nil, err + } + } + if err := resources.CreateOrUpdateResource(ctx, c, rm); err != nil { + return nil, fmt.Errorf("error creating/updating Role '%s/%s': %w", r.Namespace, r.Name, err) + } + return r, nil +} + +// EnsureRoleBinding ensures that the specified RoleBinding exists with the specified subjects. +// If it doesn't exist, it is created with the expected labels. +// If it exists, but does not have the expected labels, a ResourceNotManagedError is returned. +// The RoleBinding is returned. +func EnsureRoleBinding(ctx context.Context, c client.Client, name, namespace, roleName string, subjects []rbacv1.Subject, expectedLabels ...Label) (*rbacv1.RoleBinding, error) { + rbm := resources.NewRoleBindingMutator(name, namespace, subjects, resources.NewRoleRef(roleName), LabelListToMap(expectedLabels), nil) + rb := rbm.Empty() + found := true + if err := c.Get(ctx, client.ObjectKeyFromObject(rb), rb); err != nil { + if !apierrors.IsNotFound(err) { + return nil, fmt.Errorf("error getting RoleBinding '%s/%s': %w", rb.Namespace, rb.Name, err) + } + found = false + } + if found { + if err := FailIfNotManaged(rb, expectedLabels...); err != nil { + return nil, err + } + } + if err := resources.CreateOrUpdateResource(ctx, c, rbm); err != nil { + return nil, fmt.Errorf("error creating/updating RoleBinding '%s/%s': %w", rb.Namespace, rb.Name, err) + } + return rb, nil +} + +// CreateTokenForServiceAccount generates a token for the given ServiceAccount. +func CreateTokenForServiceAccount(ctx context.Context, c client.Client, sa *corev1.ServiceAccount, desiredDuration *time.Duration) (*ServiceAccountToken, error) { + tr := &authenticationv1.TokenRequest{} + if desiredDuration != nil { + tr.Spec.ExpirationSeconds = ptr.To((int64)(desiredDuration.Seconds())) + } + + sat := &ServiceAccountToken{ + CreationTimestamp: time.Now(), + } + if err := c.SubResource("token").Create(ctx, sa, tr); err != nil { + return nil, fmt.Errorf("error creating token for ServiceAccount '%s/%s': %w", sa.Namespace, sa.Name, err) + } + sat.Token = tr.Status.Token + sat.ExpirationTimestamp = tr.Status.ExpirationTimestamp.Time + + return sat, nil +} + +// ServiceAccountToken is a helper struct that bundles a ServiceAccount token together with its creation and expiration timestamps. +type ServiceAccountToken struct { + Token string + CreationTimestamp time.Time + ExpirationTimestamp time.Time +} + +// CreateTokenKubeconfig generates a kubeconfig based on the given values. +// The 'user' arg is used as key for the auth configuration and can be chosen freely. +func CreateTokenKubeconfig(user, host string, caData []byte, token string) ([]byte, error) { + id := "cluster" + kcfg := clientcmdapi.Config{ + APIVersion: "v1", + Kind: "Config", + Clusters: map[string]*clientcmdapi.Cluster{ + id: { + Server: host, + CertificateAuthorityData: caData, + }, + }, + Contexts: map[string]*clientcmdapi.Context{ + id: { + Cluster: id, + AuthInfo: user, + }, + }, + CurrentContext: id, + AuthInfos: map[string]*clientcmdapi.AuthInfo{ + user: { + Token: token, + }, + }, + } + + kcfgBytes, err := clientcmd.Write(kcfg) + if err != nil { + return nil, fmt.Errorf("error converting converting generated kubeconfig into yaml: %w", err) + } + return kcfgBytes, nil +} + +// ComputeTokenRenewalTime computes the time for the renewal of a token, given its creation and expiration time. +// Returns the zero time if either of the given times is zero. +// The returned time is when 80% of the validity duration is reached. +// If another percentage is desired, use ComputeTokenRenewalTimeWithRatio instead. +func ComputeTokenRenewalTime(creationTime, expirationTime time.Time) time.Time { + return ComputeTokenRenewalTimeWithRatio(creationTime, expirationTime, 0.8) +} + +// ComputeTokenRenewalTime computes the time for the renewal of a token, given its creation and expiration time. +// Returns the zero time if either of the given times is zero. +// Ratio must be between 0 and 1. The returned time is when this percentage of the validity duration is reached. +func ComputeTokenRenewalTimeWithRatio(creationTime, expirationTime time.Time, ratio float64) time.Time { + if creationTime.IsZero() || expirationTime.IsZero() { + return time.Time{} + } + // validity is how long the token was valid in the first place + validity := expirationTime.Sub(creationTime) + // renewalAfter is 80% of the validity + renewalAfter := time.Duration(float64(validity) * ratio) + // renewalAt is the point in time at which the token should be renewed + renewalAt := creationTime.Add(renewalAfter) + return renewalAt +} diff --git a/pkg/clusteraccess/access_test.go b/pkg/clusteraccess/access_test.go new file mode 100644 index 0000000..c8d9cb7 --- /dev/null +++ b/pkg/clusteraccess/access_test.go @@ -0,0 +1,390 @@ +package clusteraccess_test + +import ( + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "sigs.k8s.io/controller-runtime/pkg/client" + + corev1 "k8s.io/api/core/v1" + rbacv1 "k8s.io/api/rbac/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "github.com/openmcp-project/controller-utils/pkg/clusteraccess" + testutils "github.com/openmcp-project/controller-utils/pkg/testing" +) + +var testLabelsMap = map[string]string{ + "label1": "value1", + "label2": "value2", +} +var testLabelsList = clusteraccess.LabelMapToList(testLabelsMap) + +var _ = Describe("ClusterAccess", func() { + + Context("EnsureNamespace", func() { + + It("should create a namespace if it does not exist", func() { + env := testutils.NewEnvironmentBuilder().WithFakeClient(nil).Build() + ns := &corev1.Namespace{} + ns.SetName("testns") + Expect(env.Client().Get(env.Ctx, client.ObjectKeyFromObject(ns), ns)).To(MatchError(apierrors.IsNotFound, "namespace should not exist")) + ns, err := clusteraccess.EnsureNamespace(env.Ctx, env.Client(), ns.Name, testLabelsList...) + Expect(err).ToNot(HaveOccurred()) + Expect(env.Client().Get(env.Ctx, client.ObjectKeyFromObject(ns), ns)).To(Succeed()) + Expect(ns.Labels).To(BeEquivalentTo(testLabelsMap)) + }) + + It("should throw an error if the namespace exists, but is missing the expected labels", func() { + ns := &corev1.Namespace{} + ns.SetName("testns") + env := testutils.NewEnvironmentBuilder().WithFakeClient(nil).WithInitObjects(ns).Build() + Expect(env.Client().Get(env.Ctx, client.ObjectKeyFromObject(ns), ns)).To(Succeed()) + Expect(ns.Labels).To(BeEmpty()) + _, err := clusteraccess.EnsureNamespace(env.Ctx, env.Client(), ns.Name, testLabelsList...) + Expect(err).To(HaveOccurred()) + }) + + It("should not fail if the namespace exists and has the expected labels", func() { + ns := &corev1.Namespace{} + ns.SetName("testns") + ns.SetLabels(testLabelsMap) + env := testutils.NewEnvironmentBuilder().WithFakeClient(nil).WithInitObjects(ns).Build() + Expect(env.Client().Get(env.Ctx, client.ObjectKeyFromObject(ns), ns)).To(Succeed()) + Expect(ns.Labels).To(BeEquivalentTo(testLabelsMap)) + ns, err := clusteraccess.EnsureNamespace(env.Ctx, env.Client(), ns.Name, testLabelsList...) + Expect(err).ToNot(HaveOccurred()) + Expect(ns.Labels).To(BeEquivalentTo(testLabelsMap)) + }) + + }) + + Context("EnsureServiceAccount", func() { + + It("should create a serviceaccount if it does not exist", func() { + env := testutils.NewEnvironmentBuilder().WithFakeClient(nil).Build() + sa := &corev1.ServiceAccount{} + sa.SetName("testsa") + sa.SetNamespace("testns") + Expect(env.Client().Get(env.Ctx, client.ObjectKeyFromObject(sa), sa)).To(MatchError(apierrors.IsNotFound, "sa should not exist")) + sa, err := clusteraccess.EnsureServiceAccount(env.Ctx, env.Client(), sa.Name, sa.Namespace, testLabelsList...) + Expect(err).ToNot(HaveOccurred()) + Expect(env.Client().Get(env.Ctx, client.ObjectKeyFromObject(sa), sa)).To(Succeed()) + Expect(sa.Labels).To(BeEquivalentTo(testLabelsMap)) + }) + + It("should throw an error if the serviceaccount exists, but is missing the expected labels", func() { + sa := &corev1.ServiceAccount{} + sa.SetName("testsa") + sa.SetNamespace("testns") + env := testutils.NewEnvironmentBuilder().WithFakeClient(nil).WithInitObjects(sa).Build() + Expect(env.Client().Get(env.Ctx, client.ObjectKeyFromObject(sa), sa)).To(Succeed()) + Expect(sa.Labels).To(BeEmpty()) + _, err := clusteraccess.EnsureServiceAccount(env.Ctx, env.Client(), sa.Name, sa.Namespace, testLabelsList...) + Expect(err).To(HaveOccurred()) + }) + + It("should not fail if the serviceaccount exists and has the expected labels", func() { + sa := &corev1.ServiceAccount{} + sa.SetName("testsa") + sa.SetNamespace("testns") + sa.SetLabels(testLabelsMap) + env := testutils.NewEnvironmentBuilder().WithFakeClient(nil).WithInitObjects(sa).Build() + Expect(env.Client().Get(env.Ctx, client.ObjectKeyFromObject(sa), sa)).To(Succeed()) + Expect(sa.Labels).To(BeEquivalentTo(testLabelsMap)) + sa, err := clusteraccess.EnsureServiceAccount(env.Ctx, env.Client(), sa.Name, sa.Namespace, testLabelsList...) + Expect(err).ToNot(HaveOccurred()) + Expect(sa.Labels).To(BeEquivalentTo(testLabelsMap)) + }) + + }) + + Context("EnsureRole and EnsureClusterRole", func() { + + expectedRules := func() []rbacv1.PolicyRule { + return []rbacv1.PolicyRule{ + { + Verbs: []string{"get", "list"}, + APIGroups: []string{"apps"}, + Resources: []string{"deployments"}, + }, + { + Verbs: []string{"*"}, + APIGroups: []string{""}, + Resources: []string{"namespaces"}, + }, + } + } + + It("should create a role if it does not exist", func() { + env := testutils.NewEnvironmentBuilder().WithFakeClient(nil).Build() + r := &rbacv1.Role{} + r.SetName("testr") + r.SetNamespace("testns") + r.Rules = expectedRules() + Expect(env.Client().Get(env.Ctx, client.ObjectKeyFromObject(r), r)).To(MatchError(apierrors.IsNotFound, "role should not exist")) + r, err := clusteraccess.EnsureRole(env.Ctx, env.Client(), r.Name, r.Namespace, expectedRules(), testLabelsList...) + Expect(err).ToNot(HaveOccurred()) + Expect(env.Client().Get(env.Ctx, client.ObjectKeyFromObject(r), r)).To(Succeed()) + Expect(r.Labels).To(BeEquivalentTo(testLabelsMap)) + Expect(r.Rules).To(BeEquivalentTo(expectedRules())) + }) + + It("should throw an error if the role exists, but is missing the expected labels", func() { + r := &rbacv1.Role{} + r.SetName("testr") + r.SetNamespace("testns") + r.Rules = expectedRules() + env := testutils.NewEnvironmentBuilder().WithFakeClient(nil).WithInitObjects(r).Build() + Expect(env.Client().Get(env.Ctx, client.ObjectKeyFromObject(r), r)).To(Succeed()) + Expect(r.Labels).To(BeEmpty()) + _, err := clusteraccess.EnsureRole(env.Ctx, env.Client(), r.Name, r.Namespace, expectedRules(), testLabelsList...) + Expect(err).To(HaveOccurred()) + }) + + It("should update the rules if the role exists and has the expected labels", func() { + r := &rbacv1.Role{} + r.SetName("testr") + r.SetNamespace("testns") + r.SetLabels(testLabelsMap) + r.Rules = []rbacv1.PolicyRule{ + { + Verbs: []string{"*"}, + APIGroups: []string{"*"}, + Resources: []string{"*"}, + }, + } + env := testutils.NewEnvironmentBuilder().WithFakeClient(nil).WithInitObjects(r).Build() + Expect(env.Client().Get(env.Ctx, client.ObjectKeyFromObject(r), r)).To(Succeed()) + Expect(r.Labels).To(BeEquivalentTo(testLabelsMap)) + r, err := clusteraccess.EnsureRole(env.Ctx, env.Client(), r.Name, r.Namespace, expectedRules(), testLabelsList...) + Expect(err).ToNot(HaveOccurred()) + Expect(env.Client().Get(env.Ctx, client.ObjectKeyFromObject(r), r)).To(Succeed()) + Expect(r.Labels).To(BeEquivalentTo(testLabelsMap)) + Expect(r.Rules).To(BeEquivalentTo(expectedRules())) + }) + + It("should create a clusterrole if it does not exist", func() { + env := testutils.NewEnvironmentBuilder().WithFakeClient(nil).Build() + cr := &rbacv1.ClusterRole{} + cr.SetName("testcr") + cr.Rules = expectedRules() + Expect(env.Client().Get(env.Ctx, client.ObjectKeyFromObject(cr), cr)).To(MatchError(apierrors.IsNotFound, "clusterrole should not exist")) + cr, err := clusteraccess.EnsureClusterRole(env.Ctx, env.Client(), cr.Name, expectedRules(), testLabelsList...) + Expect(err).ToNot(HaveOccurred()) + Expect(env.Client().Get(env.Ctx, client.ObjectKeyFromObject(cr), cr)).To(Succeed()) + Expect(cr.Labels).To(BeEquivalentTo(testLabelsMap)) + Expect(cr.Rules).To(BeEquivalentTo(expectedRules())) + }) + + It("should throw an error if the clusterrole exists, but is missing the expected labels", func() { + cr := &rbacv1.ClusterRole{} + cr.SetName("testcr") + cr.Rules = expectedRules() + env := testutils.NewEnvironmentBuilder().WithFakeClient(nil).WithInitObjects(cr).Build() + Expect(env.Client().Get(env.Ctx, client.ObjectKeyFromObject(cr), cr)).To(Succeed()) + Expect(cr.Labels).To(BeEmpty()) + _, err := clusteraccess.EnsureClusterRole(env.Ctx, env.Client(), cr.Name, expectedRules(), testLabelsList...) + Expect(err).To(HaveOccurred()) + }) + + It("should update the rules if the clusterrole exists and has the expected labels", func() { + cr := &rbacv1.ClusterRole{} + cr.SetName("testcr") + cr.SetLabels(testLabelsMap) + cr.Rules = []rbacv1.PolicyRule{ + { + Verbs: []string{"*"}, + APIGroups: []string{"*"}, + Resources: []string{"*"}, + }, + } + env := testutils.NewEnvironmentBuilder().WithFakeClient(nil).WithInitObjects(cr).Build() + Expect(env.Client().Get(env.Ctx, client.ObjectKeyFromObject(cr), cr)).To(Succeed()) + Expect(cr.Labels).To(BeEquivalentTo(testLabelsMap)) + cr, err := clusteraccess.EnsureClusterRole(env.Ctx, env.Client(), cr.Name, expectedRules(), testLabelsList...) + Expect(err).ToNot(HaveOccurred()) + Expect(env.Client().Get(env.Ctx, client.ObjectKeyFromObject(cr), cr)).To(Succeed()) + Expect(cr.Labels).To(BeEquivalentTo(testLabelsMap)) + Expect(cr.Rules).To(BeEquivalentTo(expectedRules())) + }) + + }) + + Context("EnsureRoleBinding and EnsureClusterRoleBinding", func() { + + dummyRole := func() *rbacv1.Role { + return &rbacv1.Role{ + ObjectMeta: metav1.ObjectMeta{ + Name: "testrole", + Namespace: "testns", + }, + Rules: []rbacv1.PolicyRule{ + { + Verbs: []string{"*"}, + APIGroups: []string{"*"}, + Resources: []string{"*"}, + }, + }, + } + } + dummyClusterRole := func() *rbacv1.ClusterRole { + return &rbacv1.ClusterRole{ + ObjectMeta: metav1.ObjectMeta{ + Name: "testclusterrole", + }, + Rules: []rbacv1.PolicyRule{ + { + Verbs: []string{"*"}, + APIGroups: []string{"*"}, + Resources: []string{"*"}, + }, + }, + } + } + dummyServiceAccount := func() *corev1.ServiceAccount { + return &corev1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{ + Name: "testsa", + Namespace: "testns", + }, + } + } + expectedRoleRef := func() rbacv1.RoleRef { + return rbacv1.RoleRef{ + APIGroup: rbacv1.SchemeGroupVersion.Group, + Name: "testrole", + Kind: "Role", + } + } + expectedClusterRoleRef := func() rbacv1.RoleRef { + return rbacv1.RoleRef{ + APIGroup: rbacv1.SchemeGroupVersion.Group, + Name: "testclusterrole", + Kind: "ClusterRole", + } + } + expectedSubjects := func() []rbacv1.Subject { + return []rbacv1.Subject{ + { + Kind: rbacv1.ServiceAccountKind, + Name: "testsa", + Namespace: "testns", + }, + } + } + + It("should create a rolebinding if it does not exist", func() { + env := testutils.NewEnvironmentBuilder().WithFakeClient(nil).WithInitObjects(dummyRole(), dummyServiceAccount()).Build() + rb := &rbacv1.RoleBinding{} + rb.SetName("testrb") + rb.SetNamespace("testns") + rb.RoleRef = expectedRoleRef() + rb.Subjects = expectedSubjects() + Expect(env.Client().Get(env.Ctx, client.ObjectKeyFromObject(rb), rb)).To(MatchError(apierrors.IsNotFound, "rolebinding should not exist")) + rb, err := clusteraccess.EnsureRoleBinding(env.Ctx, env.Client(), rb.Name, rb.Namespace, rb.RoleRef.Name, rb.Subjects, testLabelsList...) + Expect(err).ToNot(HaveOccurred()) + Expect(env.Client().Get(env.Ctx, client.ObjectKeyFromObject(rb), rb)).To(Succeed()) + Expect(rb.Labels).To(BeEquivalentTo(testLabelsMap)) + Expect(rb.RoleRef).To(BeEquivalentTo(expectedRoleRef())) + Expect(rb.Subjects).To(BeEquivalentTo(expectedSubjects())) + }) + + It("should throw an error if the rolebinding exists, but is missing the expected labels", func() { + rb := &rbacv1.RoleBinding{} + rb.SetName("testrb") + rb.SetNamespace("testns") + rb.RoleRef = expectedRoleRef() + rb.Subjects = expectedSubjects() + env := testutils.NewEnvironmentBuilder().WithFakeClient(nil).WithInitObjects(rb, dummyRole(), dummyServiceAccount()).Build() + Expect(env.Client().Get(env.Ctx, client.ObjectKeyFromObject(rb), rb)).To(Succeed()) + Expect(rb.Labels).To(BeEmpty()) + _, err := clusteraccess.EnsureRoleBinding(env.Ctx, env.Client(), rb.Name, rb.Namespace, rb.RoleRef.Name, rb.Subjects, testLabelsList...) + Expect(err).To(HaveOccurred()) + }) + + It("should update the rolebinding if it exists and has the expected labels", func() { + rb := &rbacv1.RoleBinding{} + rb.SetName("testrb") + rb.SetNamespace("testns") + rb.SetLabels(testLabelsMap) + rb.RoleRef = rbacv1.RoleRef{ + APIGroup: rbacv1.SchemeGroupVersion.Group, + Name: "foo", + Kind: "Role", + } + rb.Subjects = []rbacv1.Subject{ + { + Kind: rbacv1.ServiceAccountKind, + Name: "bar", + Namespace: "testns", + }, + } + env := testutils.NewEnvironmentBuilder().WithFakeClient(nil).WithInitObjects(rb, dummyRole(), dummyServiceAccount()).Build() + Expect(env.Client().Get(env.Ctx, client.ObjectKeyFromObject(rb), rb)).To(Succeed()) + Expect(rb.Labels).To(BeEquivalentTo(testLabelsMap)) + rb, err := clusteraccess.EnsureRoleBinding(env.Ctx, env.Client(), rb.Name, rb.Namespace, expectedRoleRef().Name, expectedSubjects(), testLabelsList...) + Expect(err).ToNot(HaveOccurred()) + Expect(env.Client().Get(env.Ctx, client.ObjectKeyFromObject(rb), rb)).To(Succeed()) + Expect(rb.Labels).To(BeEquivalentTo(testLabelsMap)) + Expect(rb.RoleRef).To(BeEquivalentTo(expectedRoleRef())) + Expect(rb.Subjects).To(BeEquivalentTo(expectedSubjects())) + }) + + It("should create a clusterrolebinding if it does not exist", func() { + env := testutils.NewEnvironmentBuilder().WithFakeClient(nil).WithInitObjects(dummyClusterRole(), dummyServiceAccount()).Build() + crb := &rbacv1.ClusterRoleBinding{} + crb.SetName("testcrb") + crb.RoleRef = expectedClusterRoleRef() + crb.Subjects = expectedSubjects() + Expect(env.Client().Get(env.Ctx, client.ObjectKeyFromObject(crb), crb)).To(MatchError(apierrors.IsNotFound, "clusterrolebinding should not exist")) + crb, err := clusteraccess.EnsureClusterRoleBinding(env.Ctx, env.Client(), crb.Name, crb.RoleRef.Name, crb.Subjects, testLabelsList...) + Expect(err).ToNot(HaveOccurred()) + Expect(env.Client().Get(env.Ctx, client.ObjectKeyFromObject(crb), crb)).To(Succeed()) + Expect(crb.Labels).To(BeEquivalentTo(testLabelsMap)) + Expect(crb.RoleRef).To(BeEquivalentTo(expectedClusterRoleRef())) + Expect(crb.Subjects).To(BeEquivalentTo(expectedSubjects())) + }) + + It("should throw an error if the clusterrolebinding exists, but is missing the expected labels", func() { + crb := &rbacv1.ClusterRoleBinding{} + crb.SetName("testcrb") + crb.RoleRef = expectedClusterRoleRef() + crb.Subjects = expectedSubjects() + env := testutils.NewEnvironmentBuilder().WithFakeClient(nil).WithInitObjects(crb, dummyClusterRole(), dummyServiceAccount()).Build() + Expect(env.Client().Get(env.Ctx, client.ObjectKeyFromObject(crb), crb)).To(Succeed()) + Expect(crb.Labels).To(BeEmpty()) + _, err := clusteraccess.EnsureClusterRoleBinding(env.Ctx, env.Client(), crb.Name, crb.RoleRef.Name, crb.Subjects, testLabelsList...) + Expect(err).To(HaveOccurred()) + }) + + It("should update the clusterrolebinding if it exists and has the expected labels", func() { + crb := &rbacv1.ClusterRoleBinding{} + crb.SetName("testcrb") + crb.SetLabels(testLabelsMap) + crb.RoleRef = rbacv1.RoleRef{ + APIGroup: rbacv1.SchemeGroupVersion.Group, + Name: "foo", + Kind: "ClusterRole", + } + crb.Subjects = []rbacv1.Subject{ + { + Kind: rbacv1.ServiceAccountKind, + Name: "bar", + Namespace: "testns", + }, + } + env := testutils.NewEnvironmentBuilder().WithFakeClient(nil).WithInitObjects(crb, dummyClusterRole(), dummyServiceAccount()).Build() + Expect(env.Client().Get(env.Ctx, client.ObjectKeyFromObject(crb), crb)).To(Succeed()) + Expect(crb.Labels).To(BeEquivalentTo(testLabelsMap)) + crb, err := clusteraccess.EnsureClusterRoleBinding(env.Ctx, env.Client(), crb.Name, expectedClusterRoleRef().Name, expectedSubjects(), testLabelsList...) + Expect(err).ToNot(HaveOccurred()) + Expect(env.Client().Get(env.Ctx, client.ObjectKeyFromObject(crb), crb)).To(Succeed()) + Expect(crb.Labels).To(BeEquivalentTo(testLabelsMap)) + Expect(crb.RoleRef).To(BeEquivalentTo(expectedClusterRoleRef())) + Expect(crb.Subjects).To(BeEquivalentTo(expectedSubjects())) + }) + + }) + +}) diff --git a/pkg/clusteraccess/errors.go b/pkg/clusteraccess/errors.go new file mode 100644 index 0000000..b89984b --- /dev/null +++ b/pkg/clusteraccess/errors.go @@ -0,0 +1,71 @@ +package clusteraccess + +import ( + "fmt" + + "sigs.k8s.io/controller-runtime/pkg/client" +) + +// FailIfNotManaged takes an object and a list of expected labels. +// It returns an ResourceNotManagedError, if any of the expected labels is missing on the object or has a different value. +// If the object is nil or the expected labels are empty, it returns nil. +func FailIfNotManaged(obj client.Object, expectedLabels ...Label) error { + if obj == nil || len(expectedLabels) == 0 { + return nil + } + actualLabels := obj.GetLabels() + if len(actualLabels) == 0 { + return NewResourceNotManagedError(obj, expectedLabels...) + } + for _, expected := range expectedLabels { + if actual, ok := actualLabels[expected.Key]; !ok || actual != expected.Value { + return NewResourceNotManagedError(obj, expectedLabels...) + } + } + return nil +} + +// NewResourceNotManagedError creates a new ResourceNotManagedError. +func NewResourceNotManagedError(obj client.Object, expectedLabels ...Label) *ResourceNotManagedError { + SortLabels(expectedLabels) + return &ResourceNotManagedError{ + Obj: obj, + ExpectedLabels: expectedLabels, + } +} + +type ResourceNotManagedError struct { + Obj client.Object + ExpectedLabels []Label +} + +var _ error = &ResourceNotManagedError{} + +func (e *ResourceNotManagedError) Error() string { + kind := e.Obj.GetObjectKind().GroupVersionKind().Kind + if kind == "" { + kind = "resource" + } + nsMod := "" + if e.Obj.GetNamespace() != "" { + nsMod = e.Obj.GetNamespace() + "/" + } + actualLabels := make([]Label, 0, len(e.Obj.GetLabels())) + for k, v := range e.Obj.GetLabels() { + actualLabels = append(actualLabels, Label{Key: k, Value: v}) + } + // Sort the labels for consistent error messages + SortLabels(actualLabels) + return fmt.Sprintf("%s '%s%s' exists but does not contain the expected management labels %v, its actual labels are %v", kind, nsMod, e.Obj.GetName(), e.ExpectedLabels, actualLabels) +} + +// IsResourceNotManagedError returns true if the error is non-nil and of type *ResourceNotManagedError. +func IsResourceNotManagedError(err error) bool { + if err == nil { + return false + } + if _, ok := err.(*ResourceNotManagedError); ok { + return true + } + return false +} diff --git a/pkg/clusteraccess/labels.go b/pkg/clusteraccess/labels.go new file mode 100644 index 0000000..9b7a952 --- /dev/null +++ b/pkg/clusteraccess/labels.go @@ -0,0 +1,55 @@ +package clusteraccess + +import ( + "slices" + "strings" +) + +// L is a constructor for Label. +func L(key, value string) Label { + return Label{ + Key: key, + Value: value, + } +} + +// Label represents a metadata label. +// It can be constrcuted using the L() function. +type Label struct { + Key string + Value string +} + +// Compare returns -1 if l < other, 0 if l == other, and 1 if l > other. +// It is used for sorting Labels alphabetically by their keys. +func (l *Label) Compare(other Label) int { + return strings.Compare(l.Key, other.Key) +} + +// SortLabels sorts a list of Labels alphabetically by their keys. +// The sort happens in-place. +// It is not stable, but since keys are expected to be unique, that should not matter. +func SortLabels(labels []Label) { + slices.SortFunc(labels, func(a, b Label) int { + return a.Compare(b) + }) +} + +// LabelMapToList converts a map[string]string to a list of Labels. +// Note that the order of of the list is arbitrary. +func LabelMapToList(labels map[string]string) []Label { + res := make([]Label, 0, len(labels)) + for k, v := range labels { + res = append(res, Label{Key: k, Value: v}) + } + return res +} + +// LabelListToMap converts a list of Labels to a map[string]string. +func LabelListToMap(labels []Label) map[string]string { + res := make(map[string]string, len(labels)) + for _, l := range labels { + res[l.Key] = l.Value + } + return res +} diff --git a/pkg/clusteraccess/suite_test.go b/pkg/clusteraccess/suite_test.go new file mode 100644 index 0000000..d5393e2 --- /dev/null +++ b/pkg/clusteraccess/suite_test.go @@ -0,0 +1,14 @@ +package clusteraccess_test + +import ( + "testing" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +func TestConditions(t *testing.T) { + RegisterFailHandler(Fail) + + RunSpecs(t, "ClusterAccess Test Suite") +} From f65af93db49b37b6209350386cc4dffa94bae114 Mon Sep 17 00:00:00 2001 From: Johannes Aubart Date: Tue, 20 May 2025 10:48:30 +0200 Subject: [PATCH 2/3] adapt to changed resource mutators --- pkg/clusteraccess/access.go | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/pkg/clusteraccess/access.go b/pkg/clusteraccess/access.go index 4a8c4dc..e1264b0 100644 --- a/pkg/clusteraccess/access.go +++ b/pkg/clusteraccess/access.go @@ -142,7 +142,8 @@ func EnsureClusterRoleAndBinding(ctx context.Context, c client.Client, name stri // If it exists, but does not have the expected labels, a ResourceNotManagedError is returned. // The ClusterRole is returned. func EnsureClusterRole(ctx context.Context, c client.Client, name string, rules []rbacv1.PolicyRule, expectedLabels ...Label) (*rbacv1.ClusterRole, error) { - crm := resources.NewClusterRoleMutator(name, rules, LabelListToMap(expectedLabels), nil) + crm := resources.NewClusterRoleMutator(name, rules) + crm.MetadataMutator().WithLabels(LabelListToMap(expectedLabels)) cr := crm.Empty() found := true if err := c.Get(ctx, client.ObjectKeyFromObject(cr), cr); err != nil { @@ -167,7 +168,8 @@ func EnsureClusterRole(ctx context.Context, c client.Client, name string, rules // If it exists, but does not have the expected labels, a ResourceNotManagedError is returned. // The ClusterRoleBinding is returned. func EnsureClusterRoleBinding(ctx context.Context, c client.Client, name, clusterRoleName string, subjects []rbacv1.Subject, expectedLabels ...Label) (*rbacv1.ClusterRoleBinding, error) { - crbm := resources.NewClusterRoleBindingMutator(name, subjects, resources.NewClusterRoleRef(clusterRoleName), LabelListToMap(expectedLabels), nil) + crbm := resources.NewClusterRoleBindingMutator(name, subjects, resources.NewClusterRoleRef(clusterRoleName)) + crbm.MetadataMutator().WithLabels(LabelListToMap(expectedLabels)) crb := crbm.Empty() found := true if err := c.Get(ctx, client.ObjectKeyFromObject(crb), crb); err != nil { @@ -206,7 +208,8 @@ func EnsureRoleAndBinding(ctx context.Context, c client.Client, name, namespace // If it exists, but does not have the expected labels, a ResourceNotManagedError is returned. // The Role is returned. func EnsureRole(ctx context.Context, c client.Client, name, namespace string, rules []rbacv1.PolicyRule, expectedLabels ...Label) (*rbacv1.Role, error) { - rm := resources.NewRoleMutator(name, namespace, rules, LabelListToMap(expectedLabels), nil) + rm := resources.NewRoleMutator(name, namespace, rules) + rm.MetadataMutator().WithLabels(LabelListToMap(expectedLabels)) r := rm.Empty() found := true if err := c.Get(ctx, client.ObjectKeyFromObject(r), r); err != nil { @@ -231,7 +234,8 @@ func EnsureRole(ctx context.Context, c client.Client, name, namespace string, ru // If it exists, but does not have the expected labels, a ResourceNotManagedError is returned. // The RoleBinding is returned. func EnsureRoleBinding(ctx context.Context, c client.Client, name, namespace, roleName string, subjects []rbacv1.Subject, expectedLabels ...Label) (*rbacv1.RoleBinding, error) { - rbm := resources.NewRoleBindingMutator(name, namespace, subjects, resources.NewRoleRef(roleName), LabelListToMap(expectedLabels), nil) + rbm := resources.NewRoleBindingMutator(name, namespace, subjects, resources.NewRoleRef(roleName)) + rbm.MetadataMutator().WithLabels(LabelListToMap(expectedLabels)) rb := rbm.Empty() found := true if err := c.Get(ctx, client.ObjectKeyFromObject(rb), rb); err != nil { From fd48ec999884352f32abd47efbc6e9ffffdc6347 Mon Sep 17 00:00:00 2001 From: Johannes Aubart Date: Tue, 20 May 2025 13:22:54 +0200 Subject: [PATCH 3/3] move Labels type into own package --- docs/README.md | 1 + docs/libs/pairs.md | 18 +++++ pkg/clusteraccess/access.go | 15 ++-- pkg/clusteraccess/access_test.go | 3 +- pkg/clusteraccess/errors.go | 6 +- pkg/clusteraccess/labels.go | 55 ------------- pkg/pairs/pairs.go | 96 ++++++++++++++++++++++ pkg/pairs/pairs_test.go | 131 +++++++++++++++++++++++++++++++ 8 files changed, 261 insertions(+), 64 deletions(-) create mode 100644 docs/libs/pairs.md delete mode 100644 pkg/clusteraccess/labels.go create mode 100644 pkg/pairs/pairs.go create mode 100644 pkg/pairs/pairs_test.go diff --git a/docs/README.md b/docs/README.md index a9b801b..aa50132 100644 --- a/docs/README.md +++ b/docs/README.md @@ -10,6 +10,7 @@ - [Custom Resource Definitions](libs/crds.md) - [Error Handling](libs/errors.md) - [Logging](libs/logging.md) +- [Key-Value Pairs](libs/pairs.md) - [Readiness Checks](libs/readiness.md) - [Kubernetes Resource Management](libs/resource.md) - [Kubernetes Resource Status Updating](libs/status.md) diff --git a/docs/libs/pairs.md b/docs/libs/pairs.md new file mode 100644 index 0000000..4c369bf --- /dev/null +++ b/docs/libs/pairs.md @@ -0,0 +1,18 @@ +# Key-Value Pairs + +The `pkg/pairs` library contains mainly the `Pairs` type, which is a generic type representing a single key-value pair. +The `MapToPairs` and `PairsToMap` helper functions can convert between `map[K]V` and `[]Pair[K, V]`. +This is useful for example if key-value pairs are meant to be passed into a function as variadic arguments: +```go +func myFunc(labels ...Pair[string]string) { + labelMap := PairsToMap(labels) + <...> +} + +func main() { + myFunc(pairs.New("foo", "bar"), pairs.New("bar", "baz")) +} +``` + +The `Sort` and `SortStable` functions as well as the `Compare` method of `Pair` can be used to compare and sort pairs by their keys. Note that these functions will panic if the key cannot be converted into an `int64`, `float64`, `string`, or does implement the package's `Comparable` interface. +If the interface is implemented, its `Compare` implementation takes precedence over the conversion into one of the mentioned base types. diff --git a/pkg/clusteraccess/access.go b/pkg/clusteraccess/access.go index e1264b0..be6d96f 100644 --- a/pkg/clusteraccess/access.go +++ b/pkg/clusteraccess/access.go @@ -15,9 +15,12 @@ import ( "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/client" + "github.com/openmcp-project/controller-utils/pkg/pairs" "github.com/openmcp-project/controller-utils/pkg/resources" ) +type Label = pairs.Pair[string, string] + // GetTokenBasedAccess is a convenience function that wraps the flow of ensuring namespace, serviceaccount, (cluster)role(binding), and creating the token. // It returns a kubeconfig, the token with expiration timestamp, and an error if any of the steps fail. // The name will be used for all resources except the namespace (serviceaccount, (cluster)role, (cluster)rolebinding), with anything role-related additionally being prefixed with rolePrefix. @@ -85,7 +88,7 @@ func EnsureNamespace(ctx context.Context, c client.Client, nsName string, expect // a namespace does not have any spec, so we don't have to do anything, if it was found return ns, nil } - ns.SetLabels(LabelListToMap(expectedLabels)) + ns.SetLabels(pairs.PairsToMap(expectedLabels)) if err := c.Create(ctx, ns); err != nil { return nil, fmt.Errorf("error creating Namespace '%s': %w", ns.Name, err) } @@ -115,7 +118,7 @@ func EnsureServiceAccount(ctx context.Context, c client.Client, saName, saNamesp // a serviceaccount does not have any relevant spec, so we don't have to do anything, if it was found return sa, nil } - sa.SetLabels(LabelListToMap(expectedLabels)) + sa.SetLabels(pairs.PairsToMap(expectedLabels)) if err := c.Create(ctx, sa); err != nil { return nil, fmt.Errorf("error creating ServiceAccount '%s': %w", sa.Name, err) } @@ -143,7 +146,7 @@ func EnsureClusterRoleAndBinding(ctx context.Context, c client.Client, name stri // The ClusterRole is returned. func EnsureClusterRole(ctx context.Context, c client.Client, name string, rules []rbacv1.PolicyRule, expectedLabels ...Label) (*rbacv1.ClusterRole, error) { crm := resources.NewClusterRoleMutator(name, rules) - crm.MetadataMutator().WithLabels(LabelListToMap(expectedLabels)) + crm.MetadataMutator().WithLabels(pairs.PairsToMap(expectedLabels)) cr := crm.Empty() found := true if err := c.Get(ctx, client.ObjectKeyFromObject(cr), cr); err != nil { @@ -169,7 +172,7 @@ func EnsureClusterRole(ctx context.Context, c client.Client, name string, rules // The ClusterRoleBinding is returned. func EnsureClusterRoleBinding(ctx context.Context, c client.Client, name, clusterRoleName string, subjects []rbacv1.Subject, expectedLabels ...Label) (*rbacv1.ClusterRoleBinding, error) { crbm := resources.NewClusterRoleBindingMutator(name, subjects, resources.NewClusterRoleRef(clusterRoleName)) - crbm.MetadataMutator().WithLabels(LabelListToMap(expectedLabels)) + crbm.MetadataMutator().WithLabels(pairs.PairsToMap(expectedLabels)) crb := crbm.Empty() found := true if err := c.Get(ctx, client.ObjectKeyFromObject(crb), crb); err != nil { @@ -209,7 +212,7 @@ func EnsureRoleAndBinding(ctx context.Context, c client.Client, name, namespace // The Role is returned. func EnsureRole(ctx context.Context, c client.Client, name, namespace string, rules []rbacv1.PolicyRule, expectedLabels ...Label) (*rbacv1.Role, error) { rm := resources.NewRoleMutator(name, namespace, rules) - rm.MetadataMutator().WithLabels(LabelListToMap(expectedLabels)) + rm.MetadataMutator().WithLabels(pairs.PairsToMap(expectedLabels)) r := rm.Empty() found := true if err := c.Get(ctx, client.ObjectKeyFromObject(r), r); err != nil { @@ -235,7 +238,7 @@ func EnsureRole(ctx context.Context, c client.Client, name, namespace string, ru // The RoleBinding is returned. func EnsureRoleBinding(ctx context.Context, c client.Client, name, namespace, roleName string, subjects []rbacv1.Subject, expectedLabels ...Label) (*rbacv1.RoleBinding, error) { rbm := resources.NewRoleBindingMutator(name, namespace, subjects, resources.NewRoleRef(roleName)) - rbm.MetadataMutator().WithLabels(LabelListToMap(expectedLabels)) + rbm.MetadataMutator().WithLabels(pairs.PairsToMap(expectedLabels)) rb := rbm.Empty() found := true if err := c.Get(ctx, client.ObjectKeyFromObject(rb), rb); err != nil { diff --git a/pkg/clusteraccess/access_test.go b/pkg/clusteraccess/access_test.go index c8d9cb7..b79558f 100644 --- a/pkg/clusteraccess/access_test.go +++ b/pkg/clusteraccess/access_test.go @@ -11,6 +11,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "github.com/openmcp-project/controller-utils/pkg/clusteraccess" + "github.com/openmcp-project/controller-utils/pkg/pairs" testutils "github.com/openmcp-project/controller-utils/pkg/testing" ) @@ -18,7 +19,7 @@ var testLabelsMap = map[string]string{ "label1": "value1", "label2": "value2", } -var testLabelsList = clusteraccess.LabelMapToList(testLabelsMap) +var testLabelsList = pairs.MapToPairs(testLabelsMap) var _ = Describe("ClusterAccess", func() { diff --git a/pkg/clusteraccess/errors.go b/pkg/clusteraccess/errors.go index b89984b..4104f49 100644 --- a/pkg/clusteraccess/errors.go +++ b/pkg/clusteraccess/errors.go @@ -4,6 +4,8 @@ import ( "fmt" "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/openmcp-project/controller-utils/pkg/pairs" ) // FailIfNotManaged takes an object and a list of expected labels. @@ -27,7 +29,7 @@ func FailIfNotManaged(obj client.Object, expectedLabels ...Label) error { // NewResourceNotManagedError creates a new ResourceNotManagedError. func NewResourceNotManagedError(obj client.Object, expectedLabels ...Label) *ResourceNotManagedError { - SortLabels(expectedLabels) + pairs.Sort(expectedLabels) return &ResourceNotManagedError{ Obj: obj, ExpectedLabels: expectedLabels, @@ -55,7 +57,7 @@ func (e *ResourceNotManagedError) Error() string { actualLabels = append(actualLabels, Label{Key: k, Value: v}) } // Sort the labels for consistent error messages - SortLabels(actualLabels) + pairs.Sort(actualLabels) return fmt.Sprintf("%s '%s%s' exists but does not contain the expected management labels %v, its actual labels are %v", kind, nsMod, e.Obj.GetName(), e.ExpectedLabels, actualLabels) } diff --git a/pkg/clusteraccess/labels.go b/pkg/clusteraccess/labels.go deleted file mode 100644 index 9b7a952..0000000 --- a/pkg/clusteraccess/labels.go +++ /dev/null @@ -1,55 +0,0 @@ -package clusteraccess - -import ( - "slices" - "strings" -) - -// L is a constructor for Label. -func L(key, value string) Label { - return Label{ - Key: key, - Value: value, - } -} - -// Label represents a metadata label. -// It can be constrcuted using the L() function. -type Label struct { - Key string - Value string -} - -// Compare returns -1 if l < other, 0 if l == other, and 1 if l > other. -// It is used for sorting Labels alphabetically by their keys. -func (l *Label) Compare(other Label) int { - return strings.Compare(l.Key, other.Key) -} - -// SortLabels sorts a list of Labels alphabetically by their keys. -// The sort happens in-place. -// It is not stable, but since keys are expected to be unique, that should not matter. -func SortLabels(labels []Label) { - slices.SortFunc(labels, func(a, b Label) int { - return a.Compare(b) - }) -} - -// LabelMapToList converts a map[string]string to a list of Labels. -// Note that the order of of the list is arbitrary. -func LabelMapToList(labels map[string]string) []Label { - res := make([]Label, 0, len(labels)) - for k, v := range labels { - res = append(res, Label{Key: k, Value: v}) - } - return res -} - -// LabelListToMap converts a list of Labels to a map[string]string. -func LabelListToMap(labels []Label) map[string]string { - res := make(map[string]string, len(labels)) - for _, l := range labels { - res[l.Key] = l.Value - } - return res -} diff --git a/pkg/pairs/pairs.go b/pkg/pairs/pairs.go new file mode 100644 index 0000000..0c3750c --- /dev/null +++ b/pkg/pairs/pairs.go @@ -0,0 +1,96 @@ +package pairs + +import ( + "cmp" + "fmt" + "reflect" + "slices" +) + +// New creates a new Pair with the given key and value. +func New[K comparable, V any](key K, value V) Pair[K, V] { + return Pair[K, V]{Key: key, Value: value} +} + +// Pair is a generic key-value pair. +type Pair[K comparable, V any] struct { + Key K + Value V +} + +type Comparable[T any] interface { + // CompareTo returns -1 if receiver < b, 0 if receiver == b, and 1 if receiver > b. + CompareTo(other T) int +} + +// Compare returns -1 if p.Key < other.Key, 0 if p.Key == other.Key, and 1 if p.Key > other.Key. +// It will panic if K does not implement the Comparable interface and cannot be parsed to an int64, float64, or string. +func (p Pair[K, V]) CompareTo(other Pair[K, V]) int { + // check if K implements Comparable + if a, ok := any(p.Key).(Comparable[K]); ok { + return a.CompareTo(other.Key) + } + // check if *K implements Comparable + if a, ok := any(&p.Key).(Comparable[*K]); ok { + return a.CompareTo(&other.Key) + } + pKey := reflect.ValueOf(p.Key) + otherKey := reflect.ValueOf(other.Key) + // check if K is a number + if pKey.CanInt() && otherKey.CanInt() { + return cmp.Compare(pKey.Int(), otherKey.Int()) + } + if pKey.CanFloat() && otherKey.CanFloat() { + return cmp.Compare(pKey.Float(), otherKey.Float()) + } + // check if K is a string + stringType := reflect.TypeOf("") + if pKey.CanConvert(stringType) && otherKey.CanConvert(stringType) { + a := pKey.Convert(stringType).Interface().(string) + b := otherKey.Convert(stringType).Interface().(string) + return cmp.Compare(a, b) + } + panic(fmt.Sprintf("cannot compare type %T, must either implement Comparable or be a number or string", p.Key)) +} + +// Sort sorts a list of Pairs alphabetically by their keys. +// The sort happens in-place. +// Note that this uses the Compare method, which may panic if the keys are not comparable. +func Sort[K comparable, V any](pairs []Pair[K, V]) { + slices.SortFunc(pairs, func(a, b Pair[K, V]) int { + return a.CompareTo(b) + }) +} + +// SortStable sorts a list of Pairs alphabetically by their keys. +// The sort happens in-place and is stable. +// Note that this uses the Compare method, which may panic if the keys are not comparable. +func SortStable[K comparable, V any](pairs []Pair[K, V]) { + slices.SortStableFunc(pairs, func(a, b Pair[K, V]) int { + return a.CompareTo(b) + }) +} + +// MapToPairs converts a map[K]V to a []Pair[K, V]. +// Note that the order of of the list is arbitrary. +func MapToPairs[K comparable, V any](pairs map[K]V) []Pair[K, V] { + res := make([]Pair[K, V], 0, len(pairs)) + for k, v := range pairs { + res = append(res, New(k, v)) + } + return res +} + +// PairsToMap converts a []Pair[K, V] to a map[K]V. +// In case of duplicate keys, the last value will be used. +func PairsToMap[K comparable, V any](pairs []Pair[K, V]) map[K]V { + res := make(map[K]V, len(pairs)) + for _, p := range pairs { + res[p.Key] = p.Value + } + return res +} + +func (p Pair[K, V]) String() string { + return fmt.Sprintf("%v: %v", p.Key, p.Value) +} diff --git a/pkg/pairs/pairs_test.go b/pkg/pairs/pairs_test.go new file mode 100644 index 0000000..0c68421 --- /dev/null +++ b/pkg/pairs/pairs_test.go @@ -0,0 +1,131 @@ +package pairs_test + +import ( + "cmp" + "testing" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + . "github.com/onsi/gomega/gstruct" + + "github.com/openmcp-project/controller-utils/pkg/pairs" +) + +func TestConditions(t *testing.T) { + RegisterFailHandler(Fail) + + RunSpecs(t, "ClusterAccess Test Suite") +} + +type comparableIntAlias int + +var _ pairs.Comparable[comparableIntAlias] = comparableIntAlias(0) + +// Compare implements pairs.Comparable. +// This inverses the usual order. +func (c comparableIntAlias) CompareTo(other comparableIntAlias) int { + return cmp.Compare(int(c)*(-1), int(other)*(-1)) +} + +type comparableStruct struct { + val int +} + +func (c *comparableStruct) CompareTo(other *comparableStruct) int { + return cmp.Compare(c.val, other.val) +} + +var _ = Describe("Pairs", func() { + + Context("Compare", func() { + + It("should be able to compare int-based keys", func() { + type intAlias int + a := pairs.New(intAlias(1), "foo") + b := pairs.New(intAlias(2), "bar") + Expect(a.CompareTo(b)).To(Equal(-1)) + Expect(b.CompareTo(a)).To(Equal(1)) + Expect(a.CompareTo(a)).To(Equal(0)) + }) + + It("should be able to compare float-based keys", func() { + type floatAlias float64 + a := pairs.New(floatAlias(1.1), "foo") + b := pairs.New(floatAlias(1.2), "bar") + Expect(a.CompareTo(b)).To(Equal(-1)) + Expect(b.CompareTo(a)).To(Equal(1)) + Expect(a.CompareTo(a)).To(Equal(0)) + }) + + It("should be able to compare string-based keys", func() { + type stringAlias string + a := pairs.New(stringAlias("a"), "foo") + b := pairs.New(stringAlias("b"), "bar") + Expect(a.CompareTo(b)).To(Equal(-1)) + Expect(b.CompareTo(a)).To(Equal(1)) + Expect(a.CompareTo(a)).To(Equal(0)) + }) + + It("should prefer Comparable implementation over default comparison", func() { + a := pairs.New(comparableIntAlias(1), "foo") + b := pairs.New(comparableIntAlias(2), "bar") + // inversed order, because of the Compare() implementation + Expect(a.CompareTo(b)).To(Equal(1)) + Expect(b.CompareTo(a)).To(Equal(-1)) + Expect(a.CompareTo(a)).To(Equal(0)) + }) + + It("should detect if *K does implement Comparable", func() { + a := pairs.New(comparableStruct{val: 1}, "foo") + b := pairs.New(comparableStruct{val: 2}, "bar") + Expect(a.CompareTo(b)).To(Equal(-1)) + Expect(b.CompareTo(a)).To(Equal(1)) + Expect(a.CompareTo(a)).To(Equal(0)) + }) + + It("should panic if K does not implement Comparable and is not based on a comparable type", func() { + type notComparable struct{} + a := pairs.New(notComparable{}, "foo") + b := pairs.New(notComparable{}, "bar") + Expect(func() { a.CompareTo(b) }).To(Panic()) + Expect(func() { b.CompareTo(a) }).To(Panic()) + Expect(func() { a.CompareTo(a) }).To(Panic()) + }) + + }) + + Context("Conversion", func() { + + It("should convert a map to a list of pairs", func() { + src := map[string]string{ + "foo": "bar", + "baz": "asdf", + } + pairs := pairs.MapToPairs(src) + Expect(pairs).To(ConsistOf( + MatchFields(0, Fields{ + "Key": Equal("foo"), + "Value": Equal("bar"), + }), + MatchFields(0, Fields{ + "Key": Equal("baz"), + "Value": Equal("asdf"), + }), + )) + }) + + It("should convert a list of pairs to a map", func() { + src := []pairs.Pair[string, string]{ + pairs.New("foo", "foo"), + pairs.New("foo", "bar"), + pairs.New("baz", "asdf"), + } + m := pairs.PairsToMap(src) + Expect(m).To(HaveLen(2)) + Expect(m).To(HaveKeyWithValue("foo", "bar")) + Expect(m).To(HaveKeyWithValue("baz", "asdf")) + }) + + }) + +})