From 8760afd5bcd3a82d945d5dc4af60e93594abc6f9 Mon Sep 17 00:00:00 2001 From: Enrico Candino Date: Fri, 14 Nov 2025 21:45:28 +0100 Subject: [PATCH] Added `--namespace` flag to `k3kcli policy create` (#564) * added --namespace flag to policy create to actually bind the new policy to existing namespaces * fix lint * fix tests * added overwrite flag * updated cli docs * fix tests 2 * moved double quotes to single quote * fix test --- cli/cmds/cluster_create.go | 6 +-- cli/cmds/cluster_delete.go | 2 +- cli/cmds/policy_create.go | 74 ++++++++++++++++++++++++++++++-- cli/cmds/policy_delete.go | 10 +++-- docs/cli/k3kcli_policy_create.md | 2 + tests/cli_test.go | 8 ++-- 6 files changed, 87 insertions(+), 15 deletions(-) diff --git a/cli/cmds/cluster_create.go b/cli/cmds/cluster_create.go index bdf28db..c4fcdbe 100644 --- a/cli/cmds/cluster_create.go +++ b/cli/cmds/cluster_create.go @@ -115,7 +115,7 @@ func createAction(appCtx *AppContext, config *CreateConfig) func(cmd *cobra.Comm } } - logrus.Infof("Creating cluster [%s] in namespace [%s]", name, namespace) + logrus.Infof("Creating cluster '%s' in namespace '%s'", name, namespace) cluster := newCluster(name, namespace, config) @@ -138,7 +138,7 @@ func createAction(appCtx *AppContext, config *CreateConfig) func(cmd *cobra.Comm if err := client.Create(ctx, cluster); err != nil { if apierrors.IsAlreadyExists(err) { - logrus.Infof("Cluster [%s] already exists", name) + logrus.Infof("Cluster '%s' already exists", name) } else { return err } @@ -161,7 +161,7 @@ func createAction(appCtx *AppContext, config *CreateConfig) func(cmd *cobra.Comm return fmt.Errorf("failed to wait for cluster to become ready (status: %s): %w", cluster.Status.Phase, err) } - logrus.Infof("Extracting Kubeconfig for [%s] cluster", name) + logrus.Infof("Extracting Kubeconfig for '%s' cluster", name) // retry every 5s for at most 2m, or 25 times availableBackoff := wait.Backoff{ diff --git a/cli/cmds/cluster_delete.go b/cli/cmds/cluster_delete.go index 3195106..dbd6dbc 100644 --- a/cli/cmds/cluster_delete.go +++ b/cli/cmds/cluster_delete.go @@ -48,7 +48,7 @@ func delete(appCtx *AppContext) func(cmd *cobra.Command, args []string) error { namespace := appCtx.Namespace(name) - logrus.Infof("Deleting [%s] cluster in namespace [%s]", name, namespace) + logrus.Infof("Deleting '%s' cluster in namespace '%s'", name, namespace) cluster := v1beta1.Cluster{ ObjectMeta: metav1.ObjectMeta{ diff --git a/cli/cmds/policy_create.go b/cli/cmds/policy_create.go index 93a165f..7567ae6 100644 --- a/cli/cmds/policy_create.go +++ b/cli/cmds/policy_create.go @@ -21,6 +21,8 @@ type VirtualClusterPolicyCreateConfig struct { mode string labels []string annotations []string + namespaces []string + overwrite bool } func NewPolicyCreateCmd(appCtx *AppContext) *cobra.Command { @@ -45,6 +47,8 @@ func NewPolicyCreateCmd(appCtx *AppContext) *cobra.Command { cmd.Flags().StringVar(&config.mode, "mode", "shared", "The allowed mode type of the policy") cmd.Flags().StringArrayVar(&config.labels, "labels", []string{}, "Labels to add to the policy object (e.g. key=value)") cmd.Flags().StringArrayVar(&config.annotations, "annotations", []string{}, "Annotations to add to the policy object (e.g. key=value)") + cmd.Flags().StringSliceVar(&config.namespaces, "namespace", []string{}, "The namespaces where to bind the policy") + cmd.Flags().BoolVar(&config.overwrite, "overwrite", false, "Overwrite namespace binding of existing policy") return cmd } @@ -56,8 +60,11 @@ func policyCreateAction(appCtx *AppContext, config *VirtualClusterPolicyCreateCo policyName := args[0] _, err := createPolicy(ctx, client, config, policyName) + if err != nil { + return err + } - return err + return bindPolicyToNamespaces(ctx, client, config, policyName) } } @@ -75,7 +82,7 @@ func createNamespace(ctx context.Context, client client.Client, name, policyName return err } - logrus.Infof(`Creating namespace [%s]`, name) + logrus.Infof(`Creating namespace '%s'`, name) if err := client.Create(ctx, ns); err != nil { return err @@ -86,7 +93,7 @@ func createNamespace(ctx context.Context, client client.Client, name, policyName } func createPolicy(ctx context.Context, client client.Client, config *VirtualClusterPolicyCreateConfig, policyName string) (*v1beta1.VirtualClusterPolicy, error) { - logrus.Infof("Creating policy [%s]", policyName) + logrus.Infof("Creating policy '%s'", policyName) policy := &v1beta1.VirtualClusterPolicy{ ObjectMeta: metav1.ObjectMeta{ @@ -108,8 +115,67 @@ func createPolicy(ctx context.Context, client client.Client, config *VirtualClus return nil, err } - logrus.Infof("Policy [%s] already exists", policyName) + logrus.Infof("Policy '%s' already exists", policyName) } return policy, nil } + +func bindPolicyToNamespaces(ctx context.Context, client client.Client, config *VirtualClusterPolicyCreateConfig, policyName string) error { + var errs []error + + for _, namespace := range config.namespaces { + var ns v1.Namespace + if err := client.Get(ctx, types.NamespacedName{Name: namespace}, &ns); err != nil { + if apierrors.IsNotFound(err) { + logrus.Warnf(`Namespace '%s' not found, skipping`, namespace) + } else { + errs = append(errs, err) + } + + continue + } + + if ns.Labels == nil { + ns.Labels = map[string]string{} + } + + oldPolicy := ns.Labels[policy.PolicyNameLabelKey] + + // same policy found, no need to update + if oldPolicy == policyName { + logrus.Debugf(`Policy '%s' already bound to namespace '%s'`, policyName, namespace) + continue + } + + // no old policy, safe to update + if oldPolicy == "" { + if err := client.Update(ctx, &ns); err != nil { + errs = append(errs, err) + } else { + logrus.Infof(`Added policy '%s' to namespace '%s'`, policyName, namespace) + } + + continue + } + + // different policy, warn or check for overwrite flag + if oldPolicy != policyName { + if config.overwrite { + logrus.Infof(`Found policy '%s' bound to namespace '%s'. Overwriting it with '%s'`, oldPolicy, namespace, policyName) + + ns.Labels[policy.PolicyNameLabelKey] = policyName + + if err := client.Update(ctx, &ns); err != nil { + errs = append(errs, err) + } else { + logrus.Infof(`Added policy '%s' to namespace '%s'`, policyName, namespace) + } + } else { + logrus.Warnf(`Found policy '%s' bound to namespace '%s'. Skipping. To overwrite it use the --overwrite flag`, oldPolicy, namespace) + } + } + } + + return errors.Join(errs...) +} diff --git a/cli/cmds/policy_delete.go b/cli/cmds/policy_delete.go index 179c29e..5077f59 100644 --- a/cli/cmds/policy_delete.go +++ b/cli/cmds/policy_delete.go @@ -31,13 +31,17 @@ func policyDeleteAction(appCtx *AppContext) func(cmd *cobra.Command, args []stri policy.Name = name if err := client.Delete(ctx, policy); err != nil { - if apierrors.IsNotFound(err) { - logrus.Warnf("Policy not found") - } else { + if !apierrors.IsNotFound(err) { return err } + + logrus.Warnf("Policy '%s' not found", name) + + return nil } + logrus.Infof("Policy '%s' deleted", name) + return nil } } diff --git a/docs/cli/k3kcli_policy_create.md b/docs/cli/k3kcli_policy_create.md index 584e165..96cfe55 100644 --- a/docs/cli/k3kcli_policy_create.md +++ b/docs/cli/k3kcli_policy_create.md @@ -19,6 +19,8 @@ k3kcli policy create [command options] NAME -h, --help help for create --labels stringArray Labels to add to the policy object (e.g. key=value) --mode string The allowed mode type of the policy (default "shared") + --namespace strings The namespaces where to bind the policy + --overwrite Overwrite namespace binding of existing policy ``` ### Options inherited from parent commands diff --git a/tests/cli_test.go b/tests/cli_test.go index eb26f6d..a112443 100644 --- a/tests/cli_test.go +++ b/tests/cli_test.go @@ -66,7 +66,7 @@ var _ = When("using the k3kcli", Label("cli"), func() { _, stderr, err = K3kcli("cluster", "delete", clusterName) Expect(err).To(Not(HaveOccurred()), string(stderr)) - Expect(stderr).To(ContainSubstring("Deleting [%s] cluster in namespace [%s]", clusterName, clusterNamespace)) + Expect(stderr).To(ContainSubstring(`Deleting '%s' cluster in namespace '%s'`, clusterName, clusterNamespace)) // The deletion could take a bit Eventually(func() string { @@ -92,7 +92,7 @@ var _ = When("using the k3kcli", Label("cli"), func() { _, stderr, err = K3kcli("policy", "create", policyName) Expect(err).To(Not(HaveOccurred()), string(stderr)) - Expect(stderr).To(ContainSubstring("Creating policy [%s]", policyName)) + Expect(stderr).To(ContainSubstring(`Creating policy '%s'`, policyName)) stdout, stderr, err = K3kcli("policy", "list") Expect(err).To(Not(HaveOccurred()), string(stderr)) @@ -102,7 +102,7 @@ var _ = When("using the k3kcli", Label("cli"), func() { stdout, stderr, err = K3kcli("policy", "delete", policyName) Expect(err).To(Not(HaveOccurred()), string(stderr)) Expect(stdout).To(BeEmpty()) - Expect(stderr).To(BeEmpty()) + Expect(stderr).To(ContainSubstring(`Policy '%s' deleted`, policyName)) stdout, stderr, err = K3kcli("policy", "list") Expect(err).To(Not(HaveOccurred()), string(stderr)) @@ -140,7 +140,7 @@ var _ = When("using the k3kcli", Label("cli"), func() { _, stderr, err = K3kcli("cluster", "delete", clusterName) Expect(err).To(Not(HaveOccurred()), string(stderr)) - Expect(stderr).To(ContainSubstring("Deleting [%s] cluster in namespace [%s]", clusterName, clusterNamespace)) + Expect(stderr).To(ContainSubstring(`Deleting '%s' cluster in namespace '%s'`, clusterName, clusterNamespace)) }) }) })