diff --git a/go/nautobotop/api/v1alpha1/nautobot_types.go b/go/nautobotop/api/v1alpha1/nautobot_types.go index 3d7607b4e..cc89dbdda 100644 --- a/go/nautobotop/api/v1alpha1/nautobot_types.go +++ b/go/nautobotop/api/v1alpha1/nautobot_types.go @@ -32,8 +32,9 @@ type NautobotSpec struct { // +kubebuilder:default=172800 SyncIntervalSeconds int `json:"syncIntervalSeconds,omitempty"` // +kubebuilder:default=70000 - CacheMaxSize int `json:"cacheMaxSize,omitempty"` - NautobotSecretRef SecretKeySelector `json:"nautobotSecretRef,omitempty"` + CacheMaxSize int `json:"cacheMaxSize,omitempty"` + // +kubebuilder:validation:Required + NautobotSecretRef SecretKeySelector `json:"nautobotSecretRef"` NautobotServiceRef ServiceSelector `json:"nautobotServiceRef,omitempty"` DeviceTypesRef []ConfigMapRef `json:"deviceTypeRef,omitempty"` LocationTypesRef []ConfigMapRef `json:"locationTypesRef,omitempty"` diff --git a/go/nautobotop/api/v1alpha1/secret_type.go b/go/nautobotop/api/v1alpha1/secret_type.go index 24dc813a9..4745c5539 100644 --- a/go/nautobotop/api/v1alpha1/secret_type.go +++ b/go/nautobotop/api/v1alpha1/secret_type.go @@ -2,13 +2,15 @@ package v1alpha1 type SecretKeySelector struct { // The name of the Secret resource being referred to. + // +kubebuilder:validation:Required // +kubebuilder:validation:MinLength:=1 // +kubebuilder:validation:MaxLength:=253 // +kubebuilder:validation:Pattern:=^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$ - Name string `json:"name,omitempty"` + Name string `json:"name"` // The namespace of the Secret resource being referred to. - // Ignored if referent is not cluster-scoped, otherwise defaults to the namespace of the referent. + // Ignored if referent is not cluster-scoped. For this cluster-scoped + // resource, when unset it falls back to NautobotServiceRef.Namespace. // +optional // +kubebuilder:validation:MinLength:=1 // +kubebuilder:validation:MaxLength:=63 diff --git a/go/nautobotop/config/crd/bases/sync.rax.io_nautobots.yaml b/go/nautobotop/config/crd/bases/sync.rax.io_nautobots.yaml index 7a5598c1b..44c810d56 100644 --- a/go/nautobotop/config/crd/bases/sync.rax.io_nautobots.yaml +++ b/go/nautobotop/config/crd/bases/sync.rax.io_nautobots.yaml @@ -256,7 +256,8 @@ spec: namespace: description: |- The namespace of the Secret resource being referred to. - Ignored if referent is not cluster-scoped, otherwise defaults to the namespace of the referent. + Ignored if referent is not cluster-scoped. For this cluster-scoped + resource, when unset it falls back to NautobotServiceRef.Namespace. maxLength: 63 minLength: 1 pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?$ @@ -277,6 +278,8 @@ spec: minLength: 1 pattern: ^[-._a-zA-Z0-9]+$ type: string + required: + - name type: object nautobotServiceRef: properties: @@ -526,6 +529,8 @@ spec: - configMapSelector type: object type: array + required: + - nautobotSecretRef type: object status: description: NautobotStatus defines the observed state of Nautobot. diff --git a/go/nautobotop/config/samples/sync_v1alpha1_nautobot.yaml b/go/nautobotop/config/samples/sync_v1alpha1_nautobot.yaml index 3e803998b..f246f5bf8 100644 --- a/go/nautobotop/config/samples/sync_v1alpha1_nautobot.yaml +++ b/go/nautobotop/config/samples/sync_v1alpha1_nautobot.yaml @@ -6,4 +6,11 @@ metadata: app.kubernetes.io/managed-by: kustomize name: nautobot-sample spec: - # TODO(user): Add fields here + nautobotSecretRef: + name: nautobot-secret + namespace: nautobot + usernameKey: username + tokenKey: token + nautobotServiceRef: + name: nautobot-default + namespace: nautobot diff --git a/go/nautobotop/helm/crds/clients.yaml b/go/nautobotop/helm/crds/clients.yaml index 7a5598c1b..44c810d56 100644 --- a/go/nautobotop/helm/crds/clients.yaml +++ b/go/nautobotop/helm/crds/clients.yaml @@ -256,7 +256,8 @@ spec: namespace: description: |- The namespace of the Secret resource being referred to. - Ignored if referent is not cluster-scoped, otherwise defaults to the namespace of the referent. + Ignored if referent is not cluster-scoped. For this cluster-scoped + resource, when unset it falls back to NautobotServiceRef.Namespace. maxLength: 63 minLength: 1 pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?$ @@ -277,6 +278,8 @@ spec: minLength: 1 pattern: ^[-._a-zA-Z0-9]+$ type: string + required: + - name type: object nautobotServiceRef: properties: @@ -526,6 +529,8 @@ spec: - configMapSelector type: object type: array + required: + - nautobotSecretRef type: object status: description: NautobotStatus defines the observed state of Nautobot. diff --git a/go/nautobotop/internal/controller/nautobot_controller.go b/go/nautobotop/internal/controller/nautobot_controller.go index 8bd485e97..f74e8ea78 100644 --- a/go/nautobotop/internal/controller/nautobot_controller.go +++ b/go/nautobotop/internal/controller/nautobot_controller.go @@ -172,10 +172,30 @@ func (r *NautobotReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c return ctrl.Result{RequeueAfter: requeueAfter}, nil } - // Create Nautobot client + // Validate nautobotSecretRef before attempting auth + if nautobotCR.Spec.NautobotSecretRef.Name == "" { + log.Info("nautobotSecretRef.Name is not configured, skipping sync") + nautobotCR.Status.Ready = false + nautobotCR.Status.Message = "nautobotSecretRef is not configured: secret name is required" + if err := r.Status().Update(ctx, &nautobotCR); err != nil { + log.Error(err, "failed to update status") + return ctrl.Result{}, err + } + return ctrl.Result{RequeueAfter: requeueAfter}, nil + } + + // Create Nautobot client. The not-configured case (empty secret name) is + // handled by the validation above; any error here is a genuine failure + // (RBAC, wrong namespace, API error, missing keys) and must be surfaced + // so backoff and the reconcile-error metric kick in. username, token, err := r.getAuthTokenFromSecretRef(ctx, nautobotCR) if err != nil { log.Error(err, "failed to get nautobot auth token") + nautobotCR.Status.Ready = false + nautobotCR.Status.Message = fmt.Sprintf("failed to get authentication token: %v", err) + if statusErr := r.Status().Update(ctx, &nautobotCR); statusErr != nil { + log.Error(statusErr, "failed to update status after auth error") + } return ctrl.Result{}, err } nautobotURL := fmt.Sprintf("http://%s.%s.svc.cluster.local/api", nautobotCR.Spec.NautobotServiceRef.Name, nautobotCR.Spec.NautobotServiceRef.Namespace) @@ -521,30 +541,45 @@ func (r *NautobotReconciler) syncDevice(ctx context.Context, return nil } -// getAuthTokenFromSecretRef: this will fetch Nautobot auth token from the given refer. +// getAuthTokenFromSecretRef fetches the Nautobot auth token from the referenced Secret. +// If Namespace is not set on the secret ref, it falls back to NautobotServiceRef.Namespace. func (r *NautobotReconciler) getAuthTokenFromSecretRef(ctx context.Context, nautobotCR syncv1alpha1.Nautobot) (string, string, error) { - var username, token string - if nautobotCR.Spec.NautobotSecretRef.Namespace == nil || *nautobotCR.Spec.NautobotSecretRef.Namespace == "" { - return "", "", fmt.Errorf("nautobotSecretRef %q is missing a namespace", nautobotCR.Spec.NautobotSecretRef.Name) + ref := nautobotCR.Spec.NautobotSecretRef + + // Caller should have already validated Name, but be defensive + if ref.Name == "" { + return "", "", fmt.Errorf("nautobotSecretRef name is empty") + } + + // Default namespace: use NautobotServiceRef.Namespace as a fallback + // (since the CRD is cluster-scoped and has no inherent namespace) + namespace := nautobotCR.Spec.NautobotServiceRef.Namespace + if ref.Namespace != nil && *ref.Namespace != "" { + namespace = *ref.Namespace + } + + if namespace == "" { + return "", "", fmt.Errorf("nautobotSecretRef %q is missing a namespace and no fallback namespace is available", ref.Name) } + secret := &corev1.Secret{} - err := r.Get(ctx, types.NamespacedName{Name: nautobotCR.Spec.NautobotSecretRef.Name, Namespace: *nautobotCR.Spec.NautobotSecretRef.Namespace}, secret) - if err != nil { - return "", "", err + if err := r.Get(ctx, types.NamespacedName{Name: ref.Name, Namespace: namespace}, secret); err != nil { + return "", "", fmt.Errorf("failed to fetch secret %s/%s: %w", namespace, ref.Name, err) } - // Read the secret value - if valBytes, ok := secret.Data[nautobotCR.Spec.NautobotSecretRef.UsernameKey]; ok { + + var username, token string + if valBytes, ok := secret.Data[ref.UsernameKey]; ok { username = string(valBytes) } - if valBytes, ok := secret.Data[nautobotCR.Spec.NautobotSecretRef.TokenKey]; ok { + if valBytes, ok := secret.Data[ref.TokenKey]; ok { token = string(valBytes) } - if username != "" || token != "" { - return username, token, nil + if username == "" && token == "" { + return "", "", fmt.Errorf("secret keys %q/%q not found in secret %s/%s", ref.UsernameKey, ref.TokenKey, namespace, ref.Name) } - return "", "", fmt.Errorf("secret keys not found in provide secret") + return username, token, nil } // SetupWithManager sets up the controller with the Manager. diff --git a/go/nautobotop/internal/controller/nautobot_controller_test.go b/go/nautobotop/internal/controller/nautobot_controller_test.go index 62c507270..7268cf9f1 100644 --- a/go/nautobotop/internal/controller/nautobot_controller_test.go +++ b/go/nautobotop/internal/controller/nautobot_controller_test.go @@ -21,6 +21,7 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/reconcile" @@ -33,6 +34,8 @@ import ( var _ = Describe("Nautobot Controller", func() { Context("When reconciling a resource", func() { const resourceName = "test-resource" + const secretName = "nautobot-secret" + const secretNamespace = "default" ctx := context.Background() @@ -43,15 +46,41 @@ var _ = Describe("Nautobot Controller", func() { nautobot := &syncv1alpha1.Nautobot{} BeforeEach(func() { + By("creating the referenced auth Secret") + secret := &corev1.Secret{} + secretKey := types.NamespacedName{Name: secretName, Namespace: secretNamespace} + if err := k8sClient.Get(ctx, secretKey, secret); err != nil && errors.IsNotFound(err) { + secret = &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: secretName, + Namespace: secretNamespace, + }, + Data: map[string][]byte{ + "username": []byte("admin"), + "token": []byte("test-token"), + }, + } + Expect(k8sClient.Create(ctx, secret)).To(Succeed()) + } + By("creating the custom resource for the Kind Nautobot") err := k8sClient.Get(ctx, typeNamespacedName, nautobot) if err != nil && errors.IsNotFound(err) { + secretNs := secretNamespace resource := &syncv1alpha1.Nautobot{ ObjectMeta: metav1.ObjectMeta{ Name: resourceName, Namespace: "default", }, - // TODO(user): Specify other spec details if needed. + Spec: syncv1alpha1.NautobotSpec{ + // nautobotSecretRef.Name is required by the CRD schema. + NautobotSecretRef: syncv1alpha1.SecretKeySelector{ + Name: secretName, + Namespace: &secretNs, + UsernameKey: "username", + TokenKey: "token", + }, + }, } Expect(k8sClient.Create(ctx, resource)).To(Succeed()) } @@ -65,6 +94,13 @@ var _ = Describe("Nautobot Controller", func() { By("Cleanup the specific resource instance Nautobot") Expect(k8sClient.Delete(ctx, resource)).To(Succeed()) + + By("Cleanup the referenced auth Secret") + secret := &corev1.Secret{} + secretKey := types.NamespacedName{Name: secretName, Namespace: secretNamespace} + if err := k8sClient.Get(ctx, secretKey, secret); err == nil { + Expect(k8sClient.Delete(ctx, secret)).To(Succeed()) + } }) It("should successfully reconcile the resource", func() { By("Reconciling the created resource")