Skip to content

fix: Nautobot Secret Refs annotation as it is required before Creation - #2186

Open
geetikabatra wants to merge 1 commit into
rackerlabs:mainfrom
geetikabatra:geetika/nautobotsecretref
Open

fix: Nautobot Secret Refs annotation as it is required before Creation#2186
geetikabatra wants to merge 1 commit into
rackerlabs:mainfrom
geetikabatra:geetika/nautobotsecretref

Conversation

@geetikabatra

@geetikabatra geetikabatra commented Aug 4, 2026

Copy link
Copy Markdown

If nautobotSecretRef.Name is empty, the reconciler should skip gracefully: update .Status.Ready = false and .Status.Message with a clear “not configured” message, then requeue — instead of returning a raw error.

• If nautobotSecretRef.Namespace is empty but .Name is set, default to the CR’s own namespace rather than failing.

Fixes: 2183

@geetikabatra geetikabatra changed the title Fix Nautobot Secret Refs annotation as it is required before Creation fix: Nautobot Secret Refs annotation as it is required before Creation Aug 4, 2026
@geetikabatra geetikabatra self-assigned this Aug 4, 2026
@geetikabatra
geetikabatra requested a review from a team August 4, 2026 11:00
Comment on lines +553 to +558
namespace := ""
if ref.Namespace != nil && *ref.Namespace != "" {
namespace = *ref.Namespace
} else if nautobotCR.Spec.NautobotServiceRef.Namespace != "" {
namespace = nautobotCR.Spec.NautobotServiceRef.Namespace
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can be done like this, in that way we don't need else if

namespace := nautobotCR.Spec.NautobotServiceRef.Namespace
if ref.Namespace != nil && *ref.Namespace != "" {
	namespace = *ref.Namespace
}

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Will change

@cardoe

cardoe commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Code review

Found 3 issues:

1. required on NautobotSecretRef doesn't enforce a name, and breaks the sample manifest. The marker only requires the key's presence — nested name is still omitempty (unlike configMapSelector, which nests required: [name]), so nautobotSecretRef: {} still validates. And config/samples/sync_v1alpha1_nautobot.yaml has an empty spec:, so it'll now be rejected. Either update the sample or move Required onto SecretKeySelector.Name.

CacheMaxSize int `json:"cacheMaxSize,omitempty"`
// +kubebuilder:validation:Required
NautobotSecretRef SecretKeySelector `json:"nautobotSecretRef"`
NautobotServiceRef ServiceSelector `json:"nautobotServiceRef,omitempty"`

app.kubernetes.io/managed-by: kustomize
name: nautobot-sample
spec:
# TODO(user): Add fields here

2. Auth-fetch errors get swallowed as "not configured." Any getAuthTokenFromSecretRef error — RBAC, wrong namespace, API hiccup, missing keys — becomes "authentication not configured" with a nil return, hiding it from backoff and the reconcile-error metric. Inconsistent with the rest of Reconcile, which returns hard errors. Keep the soft path only for the actual not-configured case.

// Create Nautobot client
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("authentication not configured: %v", err)
if statusErr := r.Status().Update(ctx, &nautobotCR); statusErr != nil {

3. Namespace fallback contradicts the field's doc comment. SecretKeySelector.Namespace says it "defaults to the namespace of the referent," but the code falls back to NautobotServiceRef.Namespace. Defensible for a cluster-scoped CR, but update the comment (and PR 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.
// +optional
// +kubebuilder:validation:MinLength:=1
// +kubebuilder:validation:MaxLength:=63
// +kubebuilder:validation:Pattern:=^[a-z0-9]([-a-z0-9]*[a-z0-9])?$
Namespace *string `json:"namespace,omitempty"`

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

NautobotSecretRef is optional in CRD, reconciler fails when it is unset

3 participants