yoke: breaking change: disallow multi namespace flights by default an…

…d add --multi-namespaces flag
yokecd · Jan 7, 2025 · 066c74e · 066c74e
1 parent 67ff745
commit 066c74e
Show file tree

Hide file tree

Showing 3 changed files with 92 additions and 23 deletions.
diff --git a/cmd/yoke/cmd_takeoff.go b/cmd/yoke/cmd_takeoff.go
@@ -59,6 +59,7 @@ func GetTakeoffParams(settings GlobalSettings, source io.Reader, args []string)
 	flagset.BoolVar(&params.ForceConflicts, "force-conflicts", false, "force apply changes on field manager conflicts")
 	flagset.BoolVar(&params.CreateCRDs, "create-crds", true, "applies custom resource definitions found in flights")
 	flagset.BoolVar(&params.CreateNamespaces, "create-namespaces", false, "applies namespace resources found in flights")
+	flagset.BoolVar(&params.MultiNamespaces, "multi-namespaces", false, "allows releases to create resources in other namespaces than the target namespace")
 
 	flagset.BoolVar(&params.DiffOnly, "diff-only", false, "show diff between current revision and would be applied state. Does not apply anything to cluster")
 	flagset.BoolVar(&params.Color, "color", term.IsTerminal(int(os.Stdout.Fd())), "use colored output in diffs")

diff --git a/cmd/yoke/main_test.go b/cmd/yoke/main_test.go
@@ -170,7 +170,8 @@ func TestFailApplyDryRun(t *testing.T) {
 		TakeoffParams: yoke.TakeoffParams{
 			Release: "foo",
 			Flight: yoke.FlightParams{
-				Input: createBasicDeployment(t, "sample-app", "does-not-exist"),
+				Input:     createBasicDeployment(t, "sample-app", "does-not-exist"),
+				Namespace: "does-not-exist",
 			},
 		},
 	}
@@ -182,6 +183,63 @@ func TestFailApplyDryRun(t *testing.T) {
 	)
 }
 
+func TestMultiNamespaceValidation(t *testing.T) {
+	client, err := k8s.NewClientFromKubeConfig(home.Kubeconfig)
+	require.NoError(t, err)
+
+	for _, ns := range []string{"alpha", "beta"} {
+		require.NoError(t, client.EnsureNamespace(context.Background(), ns))
+		defer func() {
+			require.NoError(t, client.Clientset.CoreV1().Namespaces().Delete(context.Background(), ns, metav1.DeleteOptions{}))
+		}()
+	}
+
+	settings := GlobalSettings{KubeConfigPath: home.Kubeconfig}
+
+	makeParams := func(multiNamespace bool) TakeoffParams {
+		return TakeoffParams{
+			GlobalSettings: settings,
+			TakeoffParams: yoke.TakeoffParams{
+				Release:         "foo",
+				MultiNamespaces: multiNamespace,
+				Flight: yoke.FlightParams{
+					Input: strings.NewReader(`[
+            {
+              apiVersion: v1,
+              kind: ConfigMap,
+              metadata: {
+                name: alpha,
+                namespace: alpha,
+              },
+              data: {},
+            },
+            {
+              apiVersion: v1,
+              kind: ConfigMap,
+              metadata: {
+                name: beta,
+                namespace: beta,
+              },
+              data: {},
+            },
+          ]`),
+				},
+			},
+		}
+	}
+
+	err = TakeOff(context.Background(), makeParams(false))
+	require.ErrorContains(t, err, "Multiple namespaces detected")
+	require.ErrorContains(t, err, `namespace "alpha" does not match target namespace "default"`)
+	require.ErrorContains(t, err, `namespace "beta" does not match target namespace "default"`)
+
+	require.NoError(t, TakeOff(context.Background(), makeParams(true)))
+	require.NoError(t, Mayday(context.Background(), MaydayParams{
+		Release:        "foo",
+		GlobalSettings: settings,
+	}))
+}
+
 func TestReleaseOwnership(t *testing.T) {
 	settings := GlobalSettings{KubeConfigPath: home.Kubeconfig}
 
@@ -244,9 +302,10 @@ func TestTakeoffWithNamespace(t *testing.T) {
 		TakeoffParams: yoke.TakeoffParams{
 			Release: "foo",
 			Flight: yoke.FlightParams{
-				Input:     createBasicDeployment(t, "sample-app", "default"),
+				Input:     createBasicDeployment(t, "sample-app", namespaceName),
 				Namespace: namespaceName,
 			},
+			CreateNamespaces: true,
 		},
 	}
 
@@ -283,6 +342,7 @@ func TestTakeoffWithNamespaceResource(t *testing.T) {
 				Release:          "foo",
 				CreateNamespaces: createNamespaces,
 				Flight: yoke.FlightParams{
+					Namespace: "test-ns-resource",
 					Input: strings.NewReader(`[
 						{
 							apiVersion: v1,

diff --git a/pkg/yoke/yoke_takeoff.go b/pkg/yoke/yoke_takeoff.go
@@ -42,6 +42,7 @@ type TakeoffParams struct {
 	SkipDryRun       bool
 	DryRun           bool
 	ForceConflicts   bool
+	MultiNamespaces  bool
 	Release          string
 	Out              string
 	Flight           FlightParams
@@ -79,23 +80,37 @@ func (commander Commander) Takeoff(ctx context.Context, params TakeoffParams) er
 	internal.AddYokeMetadata(dependencies.Namespaces, params.Release)
 	internal.AddYokeMetadata(resources, params.Release)
 
-	complete := internal.DebugTimer(ctx, "looking up resource mappings")
+	targetNS := cmp.Or(params.Flight.Namespace, "default")
 
-	for _, resource := range resources {
-		mapping, err := commander.k8s.LookupResourceMapping(resource)
-		if err != nil {
-			if meta.IsNoMatchError(err) {
-				continue
+	if err := func() error {
+		defer internal.DebugTimer(ctx, "looking up resource mappings")()
+
+		var errs []error
+		for _, resource := range resources {
+			mapping, err := commander.k8s.LookupResourceMapping(resource)
+			if err != nil {
+				if meta.IsNoMatchError(err) {
+					continue
+				}
+
+				return fmt.Errorf("failed to lookup resource mapping for %s: %w", internal.Canonical(resource), err)
 			}
-			return fmt.Errorf("failed to lookup resource mapping for %s: %w", internal.Canonical(resource), err)
-		}
-		if mapping.Scope.Name() == meta.RESTScopeNameNamespace && resource.GetNamespace() == "" {
-			resource.SetNamespace(cmp.Or(params.Flight.Namespace, "default"))
+			if mapping.Scope.Name() == meta.RESTScopeNameNamespace {
+				ns := resource.GetNamespace()
+				if ns == "" {
+					resource.SetNamespace(targetNS)
+				}
+				if !params.MultiNamespaces && ns != targetNS {
+					errs = append(errs, fmt.Errorf("%s: namespace %q does not match target namespace %q", internal.Canonical(resource), ns, targetNS))
+				}
+			}
+			resource.SetOwnerReferences(params.OwnerReferences)
 		}
-		resource.SetOwnerReferences(params.OwnerReferences)
-	}
 
-	complete()
+		return xerr.MultiErrFrom("Multiple namespaces detected (if desired enable multinamespace releases)", errs...)
+	}(); err != nil {
+		return err
+	}
 
 	if params.Out != "" {
 		if params.Out == "-" {
@@ -154,14 +169,7 @@ func (commander Commander) Takeoff(ctx context.Context, params TakeoffParams) er
 		return internal.Warning("resources are the same as previous revision: skipping takeoff")
 	}
 
-	if namespace := params.Flight.Namespace; namespace != "" {
-		if err := commander.k8s.EnsureNamespace(ctx, namespace); err != nil {
-			return fmt.Errorf("failed to ensure namespace: %w", err)
-		}
-		if err := commander.k8s.WaitForReady(ctx, toUnstructuredNS(namespace), k8s.WaitOptions{Interval: params.Poll}); err != nil {
-			return fmt.Errorf("failed to wait for namespace %s to be ready: %w", namespace, err)
-		}
-	}
+	dependencies.Namespaces = append(dependencies.Namespaces, toUnstructuredNS(targetNS))
 
 	if params.CreateCRDs || params.CreateNamespaces {
 		if err := commander.applyDependencies(ctx, dependencies, params); err != nil {