fix: Nautobot Secret Refs annotation as it is required before Creation - #2186
fix: Nautobot Secret Refs annotation as it is required before Creation#2186geetikabatra wants to merge 1 commit into
Conversation
Sorry, something went wrong.
| namespace := "" | ||
| if ref.Namespace != nil && *ref.Namespace != "" { | ||
| namespace = *ref.Namespace | ||
| } else if nautobotCR.Spec.NautobotServiceRef.Namespace != "" { | ||
| namespace = nautobotCR.Spec.NautobotServiceRef.Namespace | ||
| } |
There was a problem hiding this comment.
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
}
Sorry, something went wrong.
There was a problem hiding this comment.
Thanks! Will change
Sorry, something went wrong.
Code reviewFound 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. understack/go/nautobotop/api/v1alpha1/nautobot_types.go Lines 35 to 38 in 8c23494 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. understack/go/nautobotop/internal/controller/nautobot_controller.go Lines 187 to 193 in 8c23494 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). understack/go/nautobotop/api/v1alpha1/secret_type.go Lines 10 to 16 in 8c23494 |
Sorry, something went wrong.
Reviewers
At least 1 approving review is required to merge this pull request.
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