diff --git a/cloud/sdk_client_test.go b/cloud/sdk_client_test.go index 11b0888..88be231 100644 --- a/cloud/sdk_client_test.go +++ b/cloud/sdk_client_test.go @@ -12,6 +12,7 @@ package cloud import ( "context" + "encoding/base64" "encoding/json" "net/http" "net/http/httptest" @@ -32,6 +33,18 @@ const ( ) func TestSDKClientCreateServerUsesExpectedPayload(t *testing.T) { + for name, userData := range map[string][]byte{ + "cloud-init": []byte("#cloud-config\n"), + "ignition": []byte("{\n \"ignition\": {\"version\": \"3.2.0\"}\n}\n"), + } { + t.Run(name, func(t *testing.T) { + testSDKClientCreateServerPayload(t, userData) + }) + } +} + +func testSDKClientCreateServerPayload(t *testing.T, userData []byte) { + t.Helper() var createPayload map[string]any server := newSDKTestServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { switch { @@ -71,7 +84,7 @@ func TestSDKClientCreateServerUsesExpectedPayload(t *testing.T) { SSHKeyName: "default", NetworkID: testSDKNetworkID, SecurityGroups: []string{testSDKSecurityGroup}, - UserData: []byte("#cloud-config\n"), + UserData: userData, RootVolume: RootVolumeInput{ SizeGiB: 50, PerformanceClass: "storage_premium_perf6", @@ -94,7 +107,7 @@ func TestSDKClientCreateServerUsesExpectedPayload(t *testing.T) { assertFieldAbsent(t, createPayload, "imageId") assertStringField(t, createPayload, "availabilityZone", "eu01-1") assertStringField(t, createPayload, "keypairName", "default") - assertStringField(t, createPayload, "userData", "I2Nsb3VkLWNvbmZpZwo=") + assertStringField(t, createPayload, "userData", base64.StdEncoding.EncodeToString(userData)) assertBoolField(t, createPayload, "configDrive", true) assertNestedStringField(t, createPayload, []string{"labels", "cluster"}, "test") assertNestedStringField(t, createPayload, []string{"networking", "networkId"}, testSDKNetworkID) diff --git a/cmd/manager/main.go b/cmd/manager/main.go index ca4905e..38c271a 100644 --- a/cmd/manager/main.go +++ b/cmd/manager/main.go @@ -19,7 +19,9 @@ package main import ( "crypto/tls" "flag" + "fmt" "os" + "strings" // Import all Kubernetes client auth plugins (e.g. Azure, GCP, OIDC, etc.) // to ensure that exec-entrypoint and run can make use of them. @@ -27,9 +29,11 @@ import ( "k8s.io/apimachinery/pkg/runtime" utilruntime "k8s.io/apimachinery/pkg/util/runtime" + "k8s.io/apimachinery/pkg/util/validation" clientgoscheme "k8s.io/client-go/kubernetes/scheme" clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta2" ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/cache" "sigs.k8s.io/controller-runtime/pkg/healthz" "sigs.k8s.io/controller-runtime/pkg/log/zap" "sigs.k8s.io/controller-runtime/pkg/metrics/filters" @@ -62,6 +66,7 @@ func main() { var metricsCertPath, metricsCertName, metricsCertKey string var webhookCertPath, webhookCertName, webhookCertKey string var enableLeaderElection bool + var namespace, leaderElectionNamespace string var probeAddr string var secureMetrics bool var enableHTTP2 bool @@ -72,6 +77,9 @@ func main() { flag.BoolVar(&enableLeaderElection, "leader-elect", false, "Enable leader election for controller manager. "+ "Enabling this will ensure there is only one active controller manager.") + flag.StringVar(&namespace, "namespace", "", "Namespace to watch. Leave empty to watch all namespaces.") + flag.StringVar(&leaderElectionNamespace, "leader-election-namespace", "", + "Namespace for leader election leases. Defaults to --namespace when set, otherwise the pod namespace.") flag.BoolVar(&secureMetrics, "metrics-secure", true, "If set, the metrics endpoint is served securely via HTTPS. Use --metrics-secure=false to use HTTP instead.") flag.StringVar(&webhookCertPath, "webhook-cert-path", "", "The directory that contains the webhook certificate.") @@ -158,7 +166,7 @@ func main() { metricsServerOptions.KeyName = metricsCertKey } - mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{ + managerOptions := ctrl.Options{ Scheme: scheme, Metrics: metricsServerOptions, WebhookServer: webhookServer, @@ -176,7 +184,12 @@ func main() { // if you are doing or is intended to do any operation such as perform cleanups // after the manager stops then its usage might be unsafe. // LeaderElectionReleaseOnCancel: true, - }) + } + if err := configureNamespaces(&managerOptions, namespace, leaderElectionNamespace); err != nil { + setupLog.Error(err, "Invalid namespace configuration") + os.Exit(1) + } + mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), managerOptions) if err != nil { setupLog.Error(err, "Failed to start manager") os.Exit(1) @@ -235,3 +248,27 @@ func main() { os.Exit(1) } } + +// configureNamespaces keeps the cached reads and watches in the requested +// namespace. The default lease namespace follows the watch namespace so that +// multiple namespaced managers can each elect a leader using namespaced RBAC. +func configureNamespaces(options *ctrl.Options, namespace, leaderElectionNamespace string) error { + for name, value := range map[string]string{ + "namespace": namespace, "leader-election-namespace": leaderElectionNamespace, + } { + if value == "" { + continue + } + if problems := validation.IsDNS1123Label(value); len(problems) > 0 { + return fmt.Errorf("--%s must be a valid namespace: %s", name, strings.Join(problems, "; ")) + } + } + if namespace != "" { + options.Cache.DefaultNamespaces = map[string]cache.Config{namespace: {}} + if leaderElectionNamespace == "" { + leaderElectionNamespace = namespace + } + } + options.LeaderElectionNamespace = leaderElectionNamespace + return nil +} diff --git a/cmd/manager/main_test.go b/cmd/manager/main_test.go new file mode 100644 index 0000000..e878eff --- /dev/null +++ b/cmd/manager/main_test.go @@ -0,0 +1,140 @@ +/* +Copyright 2026. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 +*/ + +package main + +import ( + "context" + "strings" + "testing" + "time" + + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/cache" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/envtest" +) + +func TestConfigureNamespaces(t *testing.T) { + for _, tt := range []struct { + name string + namespace string + leaderElectionNamespace string + wantLeaseNamespace string + wantError string + }{ + {name: "cluster wide defaults"}, + {name: "namespaced lease defaults", namespace: "clusters-example", wantLeaseNamespace: "clusters-example"}, + { + name: "explicit lease namespace", namespace: "clusters-example", + leaderElectionNamespace: "provider-system", wantLeaseNamespace: "provider-system", + }, + { + name: "cluster wide with explicit lease namespace", + leaderElectionNamespace: "provider-system", wantLeaseNamespace: "provider-system", + }, + {name: "reject multiple namespaces", namespace: "one,two", wantError: "--namespace"}, + {name: "reject whitespace", namespace: " ", wantError: "--namespace"}, + {name: "reject uppercase", namespace: "HostedCluster", wantError: "--namespace"}, + {name: "reject invalid lease namespace", leaderElectionNamespace: "one/two", wantError: "--leader-election-namespace"}, + } { + t.Run(tt.name, func(t *testing.T) { + options := ctrl.Options{} + err := configureNamespaces(&options, tt.namespace, tt.leaderElectionNamespace) + if tt.wantError != "" { + if err == nil || !strings.Contains(err.Error(), tt.wantError) { + t.Fatalf("configureNamespaces() error = %v, want %q", err, tt.wantError) + } + return + } + if err != nil { + t.Fatalf("configureNamespaces() error = %v", err) + } + if options.LeaderElectionNamespace != tt.wantLeaseNamespace { + t.Fatalf("lease namespace = %q, want %q", options.LeaderElectionNamespace, tt.wantLeaseNamespace) + } + if tt.namespace == "" { + if options.Cache.DefaultNamespaces != nil { + t.Fatal("cluster-wide manager unexpectedly restricts namespaces") + } + } else if _, ok := options.Cache.DefaultNamespaces[tt.namespace]; !ok || len(options.Cache.DefaultNamespaces) != 1 { + t.Fatalf("cache namespaces = %v, want only %q", options.Cache.DefaultNamespaces, tt.namespace) + } + }) + } +} + +// Exercise the actual controller-runtime cache: a namespaced deployment must +// neither observe nor read another hosted cluster's bootstrap or cloud secrets. +func TestNamespaceCacheIsolation(t *testing.T) { + testEnv := &envtest.Environment{} + config, err := testEnv.Start() + if err != nil { + t.Fatalf("start envtest: %v", err) + } + t.Cleanup(func() { + if err := testEnv.Stop(); err != nil { + t.Errorf("stop envtest: %v", err) + } + }) + liveClient, err := client.New(config, client.Options{Scheme: scheme}) + if err != nil { + t.Fatalf("create live client: %v", err) + } + ctx, cancel := context.WithTimeout(t.Context(), 30*time.Second) + defer cancel() + for _, namespace := range []string{"hosted-one", "hosted-two"} { + if err := liveClient.Create(ctx, &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: namespace}}); err != nil { + t.Fatalf("create namespace: %v", err) + } + if err := liveClient.Create(ctx, &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Namespace: namespace, Name: "credentials"}, + Data: map[string][]byte{"value": []byte(namespace)}, + }); err != nil { + t.Fatalf("create secret: %v", err) + } + } + options := ctrl.Options{} + if err := configureNamespaces(&options, "hosted-one", ""); err != nil { + t.Fatal(err) + } + options.Cache.Scheme = scheme + scopedCache, err := cache.New(config, options.Cache) + if err != nil { + t.Fatalf("create namespace cache: %v", err) + } + cacheErrors := make(chan error, 1) + go func() { cacheErrors <- scopedCache.Start(ctx) }() + t.Cleanup(func() { + cancel() + if err := <-cacheErrors; err != nil { + t.Errorf("run cache: %v", err) + } + }) + secret := &corev1.Secret{} + if err := scopedCache.Get(ctx, client.ObjectKey{Namespace: "hosted-one", Name: "credentials"}, secret); err != nil { + t.Fatalf("get watched secret: %v", err) + } + if string(secret.Data["value"]) != "hosted-one" { + t.Fatalf("read wrong namespace's secret: %s", secret.Namespace) + } + if err := scopedCache.Get(ctx, client.ObjectKey{Namespace: "hosted-two", Name: "credentials"}, &corev1.Secret{}); err == nil { + t.Fatal("cache permitted a secret read outside its namespace") + } + secrets := &corev1.SecretList{} + if err := scopedCache.List(ctx, secrets); err != nil { + t.Fatalf("list watched secrets: %v", err) + } + if len(secrets.Items) != 1 || secrets.Items[0].Namespace != "hosted-one" { + t.Fatalf("cache list escaped namespace scope: %v", secrets.Items) + } +} diff --git a/controller/controller_test_helpers_test.go b/controller/controller_test_helpers_test.go index d103137..4d1ed7e 100644 --- a/controller/controller_test_helpers_test.go +++ b/controller/controller_test_helpers_test.go @@ -32,11 +32,11 @@ const ( testImageID = "33333333-3333-3333-3333-333333333333" ) -func createCredentialsSecret(ctx context.Context, name, namespace, projectID string) { +func createCredentialsSecret(ctx context.Context, name string) { secret := &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: namespace}, + ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: "default"}, Data: map[string][]byte{ - "project-id": []byte(projectID), + "project-id": []byte(testProjectID), "serviceaccount.json": []byte("{}"), }, } diff --git a/controller/stackitcluster_controller.go b/controller/stackitcluster_controller.go index 2c4be47..026fcff 100644 --- a/controller/stackitcluster_controller.go +++ b/controller/stackitcluster_controller.go @@ -27,6 +27,7 @@ import ( "k8s.io/client-go/tools/events" clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta2" clusterutil "sigs.k8s.io/cluster-api/util" + "sigs.k8s.io/cluster-api/util/annotations" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/handler" @@ -69,6 +70,12 @@ func (r *StackitClusterReconciler) Reconcile(ctx context.Context, req ctrl.Reque } return ctrl.Result{}, err } + // An external controller owns this infrastructure object's lifecycle and + // status. Do not add finalizers, patch conditions, or access cloud resources. + if annotations.IsExternallyManaged(stackitCluster) { + log.V(1).Info("Skipping externally managed StackitCluster") + return ctrl.Result{}, nil + } cluster, err := clusterutil.GetOwnerCluster(ctx, r.Client, stackitCluster.ObjectMeta) if err != nil { diff --git a/controller/stackitcluster_controller_test.go b/controller/stackitcluster_controller_test.go index 1ca1de0..bfc421b 100644 --- a/controller/stackitcluster_controller_test.go +++ b/controller/stackitcluster_controller_test.go @@ -59,7 +59,7 @@ var _ = Describe("StackitCluster Controller", func() { }, } - createCredentialsSecret(ctx, credentials, namespace, testProjectID) + createCredentialsSecret(ctx, credentials) createOwnerCluster(ctx, clusterName) stackitClust = newStackitCluster(clusterName, namespace, true) stackitClust.Spec.CredentialsSecretRef.Name = credentials @@ -116,6 +116,40 @@ var _ = Describe("StackitCluster Controller", func() { expectCondition(got.Status.Conditions, infrav1.ClusterLoadBalancerReadyCondition, metav1.ConditionTrue, "Skipped") }) + DescribeTable("leaves externally managed infrastructure untouched", func(managedBy string, deleting bool) { + got := &infrav1.StackitCluster{} + Expect(k8sClient.Get(ctx, stackitKey, got)).To(Succeed()) + got.Annotations = map[string]string{clusterv1.ManagedByAnnotation: managedBy} + if deleting { + // Keep the object observable while deletion is in progress. The + // external controller is responsible for its own finalizers. + got.Finalizers = []string{"external.example.com/cleanup"} + } + Expect(k8sClient.Update(ctx, got)).To(Succeed()) + got.Status.Ready = true + got.Status.Initialization.Provisioned = true + Expect(k8sClient.Status().Update(ctx, got)).To(Succeed()) + if deleting { + Expect(k8sClient.Delete(ctx, got)).To(Succeed()) + } + Expect(k8sClient.Get(ctx, stackitKey, got)).To(Succeed()) + before := got.DeepCopy() + reconciler.CloudClientFactory = func(context.Context, cloud.Credentials) (cloud.Client, error) { + Fail("externally managed clusters must not access cloud resources") + return nil, nil + } + + result, err := reconciler.Reconcile(ctx, request) + Expect(err).NotTo(HaveOccurred()) + Expect(result).To(Equal(reconcile.Result{})) + Expect(k8sClient.Get(ctx, stackitKey, got)).To(Succeed()) + Expect(got).To(Equal(before)) + }, + Entry("during normal reconciliation", "external.example.com/controller", false), + Entry("when the annotation value is empty", "", false), + Entry("during deletion", "external.example.com/controller", true), + ) + It("creates the bastion and publishes its public IP when enabled", func() { got := &infrav1.StackitCluster{} Expect(k8sClient.Get(ctx, stackitKey, got)).To(Succeed()) @@ -243,40 +277,59 @@ var _ = Describe("StackitCluster Controller", func() { Expect(fakeCloud.SecurityGroupCount()).To(Equal(0)) }) - It("finalizes deletion when the credentials Secret is already gone", func() { - // The Secret commonly disappears first during namespace teardown, and - // without it no cloud client can be built at all. - createOwnerCluster(ctx, clusterName+"-nocreds") - defer deleteIfExists(ctx, &clusterv1.Cluster{ - ObjectMeta: metav1.ObjectMeta{Name: clusterName + "-nocreds", Namespace: namespace}, - }) - orphaned := newStackitCluster(clusterName+"-nocreds", namespace, false) - orphaned.Spec.CredentialsSecretRef.Name = credentials - orphaned.Spec.Bastion = validBastionSpec() - Expect(k8sClient.Create(ctx, orphaned)).To(Succeed()) - defer deleteIfExists(ctx, orphaned) - - key := types.NamespacedName{Namespace: namespace, Name: orphaned.Name} - req := reconcile.Request{NamespacedName: key} - _, err := reconciler.Reconcile(ctx, req) + DescribeTable("retains cluster resources until credentials are restored", func(missing bool) { + got := &infrav1.StackitCluster{} + Expect(k8sClient.Get(ctx, stackitKey, got)).To(Succeed()) + got.Spec.Bastion = validBastionSpec() + Expect(k8sClient.Update(ctx, got)).To(Succeed()) + _, err := reconciler.Reconcile(ctx, request) Expect(err).NotTo(HaveOccurred()) - By("removing the credentials Secret, as namespace teardown would") - Expect(k8sClient.Delete(ctx, &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{Name: credentials, Namespace: namespace}, - })).To(Succeed()) - - got := &infrav1.StackitCluster{} - Expect(k8sClient.Get(ctx, key, got)).To(Succeed()) + secret := &corev1.Secret{} + Expect(k8sClient.Get(ctx, types.NamespacedName{Namespace: namespace, Name: credentials}, secret)).To(Succeed()) + if missing { + Expect(k8sClient.Delete(ctx, secret)).To(Succeed()) + } else { + secret.Data = nil + Expect(k8sClient.Update(ctx, secret)).To(Succeed()) + } + Expect(k8sClient.Get(ctx, stackitKey, got)).To(Succeed()) Expect(k8sClient.Delete(ctx, got)).To(Succeed()) - _, err = reconciler.Reconcile(ctx, req) - Expect(err).NotTo(HaveOccurred(), "deletion must not block on a Secret that can never come back") + result, err := reconciler.Reconcile(ctx, request) + if missing { + Expect(apierrors.IsNotFound(err)).To(BeTrue()) + } else { + Expect(err).NotTo(HaveOccurred()) + Expect(result.RequeueAfter).To(Equal(credentialsRetryRequeueAfter)) + } + Expect(k8sClient.Get(ctx, stackitKey, got)).To(Succeed()) + Expect(got.Finalizers).To(ContainElement(infrav1.ClusterFinalizer)) + expectCondition(got.Status.Conditions, infrav1.ClusterCredentialsReadyCondition, metav1.ConditionFalse, "CredentialsInvalid") + Expect(fakeCloud.LoadBalancerCount()).To(Equal(1)) + Expect(fakeCloud.ServerCount()).To(Equal(1)) + Expect(fakeCloud.PublicIPCount()).To(Equal(1)) + Expect(fakeCloud.SecurityGroupCount()).To(Equal(1)) + + By("restoring credentials and completing cleanup") + deleteIfExists(ctx, secret) + createCredentialsSecret(ctx, credentials) + _, err = reconciler.Reconcile(ctx, request) + Expect(err).NotTo(HaveOccurred()) + Expect(fakeCloud.LoadBalancerCount()).To(Equal(0)) + Expect(fakeCloud.ServerCount()).To(Equal(0)) + Expect(fakeCloud.PublicIPCount()).To(Equal(0)) + Expect(fakeCloud.SecurityGroupCount()).To(Equal(0)) + _, err = fakeCloud.GetNetwork(ctx, testNetworkID) + Expect(err).NotTo(HaveOccurred(), "cluster cleanup must preserve the user-owned network") Eventually(func() bool { - return apierrors.IsNotFound(k8sClient.Get(ctx, key, &infrav1.StackitCluster{})) - }).Should(BeTrue(), "cluster stayed in Terminating because the finalizer was never removed") - }) + return apierrors.IsNotFound(k8sClient.Get(ctx, stackitKey, &infrav1.StackitCluster{})) + }).Should(BeTrue()) + }, + Entry("when the Secret is missing", true), + Entry("when the Secret is invalid", false), + ) It("tears the bastion down when disabled even if its status was never persisted", func() { // With the status lost, a status-gated teardown would report the bastion diff --git a/controller/stackitcluster_infrastructure.go b/controller/stackitcluster_infrastructure.go index 2ba49c8..f0b77df 100644 --- a/controller/stackitcluster_infrastructure.go +++ b/controller/stackitcluster_infrastructure.go @@ -17,7 +17,6 @@ import ( "time" corev1 "k8s.io/api/core/v1" - apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta2" "sigs.k8s.io/cluster-api/util/collections" @@ -253,28 +252,15 @@ func (r *StackitClusterReconciler) reconcileDelete(ctx context.Context, clusterS // DeleteNodeSSHAccess fall back to tag lookups and tolerate NotFound. cloudClient, err := util.BuildCloudClient(ctx, r.Client, r.CloudClientFactory, stackitCluster) if err != nil { - // A missing credentials Secret cannot be recovered from and commonly - // disappears first during namespace teardown, so finalize and make the - // possible leak loud rather than stranding the cluster in Terminating. - // Every other credentials problem is fixable and keeps retrying. - if apierrors.IsNotFound(err) { - if r.Recorder != nil { - r.Recorder.Eventf(stackitCluster, nil, corev1.EventTypeWarning, "CleanupSkipped", "Delete", - "Credentials Secret is gone; finalizing without cloud cleanup. "+ - "Any remaining STACKIT resources for this cluster must be removed manually: %v", err) - } - controllerutil.RemoveFinalizer(stackitCluster, infrav1.ClusterFinalizer) - return ctrl.Result{}, nil - } - util.SetConditions( + // A missing or invalid Secret can be restored. Retain the finalizer + // until the cloud confirms that provider-owned resources are removed. + return util.CredentialFailureResult( &stackitCluster.Status.Conditions, stackitCluster.Generation, - metav1.ConditionFalse, - "CredentialsInvalid", - err.Error(), + err, + credentialsRetryRequeueAfter, infrav1.ClusterCredentialsReadyCondition, ) - return ctrl.Result{}, err } loadBalancerID, err := loadbalancerservice.ResolveID(ctx, cloudClient, stackitCluster) if err != nil { diff --git a/controller/stackitmachine_controller.go b/controller/stackitmachine_controller.go index cc91f21..1789689 100644 --- a/controller/stackitmachine_controller.go +++ b/controller/stackitmachine_controller.go @@ -111,7 +111,7 @@ func (r *StackitMachineReconciler) Reconcile(ctx context.Context, req ctrl.Reque util.SetPausedCondition(&stackitMachine.Status.Conditions, stackitMachine.Generation, false, "") if !stackitMachine.DeletionTimestamp.IsZero() { - return ctrl.Result{}, r.reconcileDelete(ctx, machineScope) + return r.reconcileDelete(ctx, machineScope) } return r.reconcileNormal(ctx, machineScope) } diff --git a/controller/stackitmachine_controller_test.go b/controller/stackitmachine_controller_test.go index 968f5f9..4f221d6 100644 --- a/controller/stackitmachine_controller_test.go +++ b/controller/stackitmachine_controller_test.go @@ -67,7 +67,7 @@ var _ = Describe("StackitMachine Controller", func() { }, } - createCredentialsSecret(ctx, credentials, namespace, testProjectID) + createCredentialsSecret(ctx, credentials) createOwnerCluster(ctx, clusterName) createReadyStackitCluster(ctx, clusterName, namespace, credentials) createOwnerMachine(ctx, machineName, clusterName, stackitName) @@ -141,6 +141,22 @@ var _ = Describe("StackitMachine Controller", func() { expectCondition(got.Status.Conditions, infrav1.MachineInstanceReadyCondition, metav1.ConditionTrue, "Available") }) + It("passes Ignition bootstrap data to the server without modification", func() { + ignition := []byte("{\n \"ignition\": {\"version\": \"3.2.0\", \"config\": {\"merge\": [{\"source\": \"https://ignition.example.com/worker\"}]}}\n}\n") + secret := &corev1.Secret{} + Expect(k8sClient.Get(ctx, types.NamespacedName{Namespace: namespace, Name: bootstrapName}, secret)).To(Succeed()) + secret.Data = map[string][]byte{"value": ignition, "format": []byte("ignition")} + Expect(k8sClient.Update(ctx, secret)).To(Succeed()) + + result, err := reconciler.Reconcile(ctx, request) + Expect(err).NotTo(HaveOccurred()) + Expect(result).To(Equal(reconcile.Result{})) + got := &infrav1.StackitMachine{} + Expect(k8sClient.Get(ctx, stackitKey, got)).To(Succeed()) + Expect(got.Status.Ready).To(BeTrue()) + Expect(fakeCloud.ServerUserData(got.Status.InstanceID)).To(Equal(ignition)) + }) + It("does not silently recreate the server of an already-provisioned machine", func() { // Recreating it would replay bootstrap data pinned to the previous // identity: the replacement either never rejoins or rejoins while Machine @@ -498,6 +514,49 @@ var _ = Describe("StackitMachine Controller", func() { }).Should(BeTrue()) }) + DescribeTable("retains the server until credentials are restored", func(missing bool) { + _, err := reconciler.Reconcile(ctx, request) + Expect(err).NotTo(HaveOccurred()) + secret := &corev1.Secret{} + Expect(k8sClient.Get(ctx, types.NamespacedName{Namespace: namespace, Name: credentials}, secret)).To(Succeed()) + if missing { + Expect(k8sClient.Delete(ctx, secret)).To(Succeed()) + } else { + secret.Data = nil + Expect(k8sClient.Update(ctx, secret)).To(Succeed()) + } + got := &infrav1.StackitMachine{} + Expect(k8sClient.Get(ctx, stackitKey, got)).To(Succeed()) + Expect(k8sClient.Delete(ctx, got)).To(Succeed()) + + result, err := reconciler.Reconcile(ctx, request) + if missing { + Expect(apierrors.IsNotFound(err)).To(BeTrue()) + } else { + Expect(err).NotTo(HaveOccurred()) + Expect(result.RequeueAfter).To(Equal(credentialsRetryRequeueAfter)) + } + Expect(k8sClient.Get(ctx, stackitKey, got)).To(Succeed()) + Expect(got.Finalizers).To(ContainElement(infrav1.MachineFinalizer)) + expectCondition(got.Status.Conditions, infrav1.MachineCredentialsReadyCondition, metav1.ConditionFalse, "CredentialsInvalid") + Expect(fakeCloud.ServerCount()).To(Equal(1)) + + By("restoring credentials and completing server cleanup") + deleteIfExists(ctx, secret) + createCredentialsSecret(ctx, credentials) + _, err = reconciler.Reconcile(ctx, request) + Expect(err).NotTo(HaveOccurred()) + Expect(fakeCloud.ServerCount()).To(Equal(0)) + _, err = fakeCloud.GetNetwork(ctx, testNetworkID) + Expect(err).NotTo(HaveOccurred(), "machine cleanup must preserve the user-owned network") + Eventually(func() bool { + return apierrors.IsNotFound(k8sClient.Get(ctx, stackitKey, &infrav1.StackitMachine{})) + }).Should(BeTrue()) + }, + Entry("when the Secret is missing", true), + Entry("when the Secret is invalid", false), + ) + It("keeps the finalizer when the server deletion fails", func() { _, err := reconciler.Reconcile(ctx, request) Expect(err).NotTo(HaveOccurred()) diff --git a/controller/stackitmachine_infrastructure.go b/controller/stackitmachine_infrastructure.go index 114ff9d..5d5482e 100644 --- a/controller/stackitmachine_infrastructure.go +++ b/controller/stackitmachine_infrastructure.go @@ -183,46 +183,34 @@ func validateMachineAvailabilityZone(machineScope *scope.MachineScope) error { return fmt.Errorf("availabilityZone %q is not published in StackitCluster status.failureDomains", availabilityZone) } -func (r *StackitMachineReconciler) reconcileDelete(ctx context.Context, machineScope *scope.MachineScope) error { +func (r *StackitMachineReconciler) reconcileDelete(ctx context.Context, machineScope *scope.MachineScope) (ctrl.Result, error) { stackitMachine := machineScope.StackitMachine // An empty status.instanceID is not proof that no server exists, so every // deletion asks the cloud before dropping the finalizer. cloudClient, err := util.BuildCloudClient(ctx, r.Client, r.CloudClientFactory, machineScope.StackitCluster) if err != nil { - // A missing credentials Secret cannot be recovered from and commonly - // disappears first during namespace teardown, so finalize and make the - // possible leak loud rather than stranding the Machine in Terminating. - // Every other credentials problem is fixable and keeps retrying. - if apierrors.IsNotFound(err) { - if r.Recorder != nil { - r.Recorder.Eventf(stackitMachine, nil, corev1.EventTypeWarning, "CleanupSkipped", "Delete", - "Credentials Secret is gone; finalizing without cloud cleanup. "+ - "Any remaining STACKIT server for this machine must be removed manually: %v", err) - } - controllerutil.RemoveFinalizer(stackitMachine, infrav1.MachineFinalizer) - return nil - } - _, resultErr := util.CredentialFailureResult( + // Credentials can be restored. Keep the finalizer until cloud cleanup + // succeeds, and propagate timed retries for invalid credentials too. + return util.CredentialFailureResult( &stackitMachine.Status.Conditions, stackitMachine.Generation, err, credentialsRetryRequeueAfter, infrav1.MachineCredentialsReadyCondition, ) - return resultErr } if err := r.deleteAPIServerLoadBalancerTarget(ctx, cloudClient, machineScope); err != nil { - return err + return ctrl.Result{}, err } instanceID, err := r.resolveServerForDeletion(ctx, cloudClient, machineScope) if err != nil { - return err + return ctrl.Result{}, err } if instanceID != "" { if err := cloudClient.DeleteServer(ctx, instanceID); err != nil && !cloud.IsNotFound(err) { - return err + return ctrl.Result{}, err } machineScope.ClearInstance() if r.Recorder != nil { @@ -232,7 +220,7 @@ func (r *StackitMachineReconciler) reconcileDelete(ctx context.Context, machineS } } controllerutil.RemoveFinalizer(stackitMachine, infrav1.MachineFinalizer) - return nil + return ctrl.Result{}, nil } // resolveServerForDeletion reports the ID of the server backing this machine, or diff --git a/docs/src/SUMMARY.md b/docs/src/SUMMARY.md index cc690bd..620ef5c 100644 --- a/docs/src/SUMMARY.md +++ b/docs/src/SUMMARY.md @@ -15,6 +15,7 @@ - [OS Images](./topics/images.md) - [Accessing VM instances](./topics/accessing-vm-instances.md) - [Failure domains](./topics/failure-domains.md) + - [Hosted control planes](./topics/hosted-control-planes.md) - [IAM Permissions Used](./topics/iam-permissions.md) diff --git a/docs/src/topics/hosted-control-planes.md b/docs/src/topics/hosted-control-planes.md new file mode 100644 index 0000000..88e82e3 --- /dev/null +++ b/docs/src/topics/hosted-control-planes.md @@ -0,0 +1,80 @@ +# Hosted control planes + +CAPSTK can provision worker machines for an externally hosted control plane. +The hosting controller supplies the Cluster API `Cluster`, `Machine` resources, +and bootstrap Secrets. CAPSTK uses the Cluster API **v1beta2** contract; the +STACKIT infrastructure resources retain their **v1alpha1** API version. + +## Run a manager per namespace + +Use `--namespace` to limit every controller cache and watch, including Secrets, +to the hosted cluster's management namespace. Run the manager with a service +account whose Role and RoleBinding grant access only in that namespace. Cache +scoping complements Kubernetes RBAC; it does not replace RBAC. + +```yaml +containers: +- name: manager + image: + args: + - --namespace=$(POD_NAMESPACE) + - --leader-elect + - --health-probe-bind-address=:8081 + - --metrics-bind-address=0 + env: + - name: POD_NAMESPACE + valueFrom: + fieldRef: + fieldPath: metadata.namespace + - name: ENABLE_WEBHOOKS + value: "false" +``` + +With `--namespace` set, leader election leases default to that namespace. Use +`--leader-election-namespace` to override the lease namespace if needed. The +health and readiness endpoints are `/healthz` and `/readyz` on the configured +probe address. Without `--namespace`, existing cluster-wide behavior remains. + +Disabling webhooks prevents this manager from starting an admission server or +requiring serving certificates. Install the CRDs separately. Admission webhook +configurations, if installed, must still point to a running admission server; +do not point them at a manager with `ENABLE_WEBHOOKS=false`. + +Credentials and bootstrap Secrets must be in the watched namespace. Set +`StackitCluster.spec.credentialsSecretRef.namespace` to that namespace or omit +it. The credentials Secret requires `serviceaccount.json`; its optional +`project-id` key must match `StackitCluster.spec.projectID` when supplied. + +## Reuse the hosted API endpoint + +Configure a `StackitCluster` with the existing worker network, hosted API +endpoint, and `spec.apiServerLoadBalancer.enabled: false`. Leave +`spec.bastion.enabled` false when no CAPSTK-managed bastion is required. CAPSTK +validates the network and credentials and publishes infrastructure readiness +without creating an API server load balancer or control-plane machines. + +An external infrastructure controller can instead own the `StackitCluster` +lifecycle using the `cluster.x-k8s.io/managed-by` annotation. In that case CAPSTK +does not change the cluster, including its finalizers or status, or manage its +cloud resources. The external controller must populate `status.ready` and +`status.initialization.provisioned` for worker provisioning and CAPI readiness, +and keep the cluster and credentials available until its machines are deleted. +Do not annotate an existing CAPSTK-managed cluster without also transferring +responsibility for its cloud resources and finalizers. + +## Bootstrap worker machines + +Point each `Machine.spec.bootstrap.dataSecretName` at a Secret containing the +bootstrap document in `data.value` (or `data.userData` for legacy consumers). +CAPSTK treats the bytes as opaque: cloud-init and Ignition JSON are forwarded +unchanged, then base64-encoded once for the STACKIT API. CAPSTK does not render, +merge, or inject configuration into the document. + +Select an image that supports the bootstrap format and STACKIT's metadata +service or config drive. A native `stackit` CoreOS image also needs the STACKIT +providers in Ignition and Afterburn; changing the image's platform ID does not +add those providers. Track image availability in the +[Fedora CoreOS platform request](https://github.com/coreos/fedora-coreos-tracker/issues/2175). +The control-plane integration remains responsible for generating the matching +bootstrap data, configuring the node's cloud provider and networking, and +validating the chosen image with an end-to-end worker lifecycle test. diff --git a/docs/src/usage/cleanup.md b/docs/src/usage/cleanup.md index 51c78d6..67f1fb6 100644 --- a/docs/src/usage/cleanup.md +++ b/docs/src/usage/cleanup.md @@ -14,6 +14,20 @@ kubectl get machine,stackitmachine,stackitcluster \ -l "cluster.x-k8s.io/cluster-name=${CLUSTER_NAME}" ``` +Keep the credentials Secret until all `StackitMachine` and `StackitCluster` +resources have finished deleting. Missing or invalid credentials leave these +resources in `Terminating` with their finalizers intact and a +`CredentialsReady=False` condition. The controllers retry cleanup once valid +credentials are restored; they do not abandon running cloud resources by +removing finalizers after a credential error. + +Delete the Cluster before deleting its management namespace. Kubernetes does +not allow creating a replacement Secret in a terminating namespace, so restore +credentials before namespace deletion. If recovery requires manual cleanup, +verify that all provider-owned resources have been removed before explicitly +removing their finalizers. Existing networks supplied to CAPSTK are user-owned +and are preserved during cluster deletion. + Real e2e resources are labeled so they can be cleaned up directly through the STACKIT API if Kubernetes cleanup fails: diff --git a/util/bootstrap.go b/util/bootstrap.go index 179ebe9..5034819 100644 --- a/util/bootstrap.go +++ b/util/bootstrap.go @@ -26,10 +26,10 @@ const ( // does not contain one of the expected keys. var ErrBootstrapDataInvalid = errors.New("bootstrap data invalid: secret has no \"value\" or \"userData\" key") -// ExtractBootstrapData returns the bootstrap payload from a Secret produced -// by CABPK/KubeadmConfig. Per spec section 13.2 it checks "value" first, then -// "userData". It returns ErrBootstrapDataInvalid when neither key is present -// or both are empty. +// ExtractBootstrapData returns an opaque bootstrap payload from a Secret. +// Both cloud-init and Ignition documents are passed through unchanged. It +// checks "value" first, then "userData", and returns ErrBootstrapDataInvalid +// when neither key is present or both are empty. func ExtractBootstrapData(secret *corev1.Secret) ([]byte, error) { if data, ok := secret.Data["value"]; ok && len(data) > 0 { return data, nil