Check error when checking for secret existence#95
Check error when checking for secret existence#95mangelajo merged 1 commit intomainjumpstarter-dev/jumpstarter-controller:mainfrom token-followupjumpstarter-dev/jumpstarter-controller:token-followupCopy head branch name to clipboard
Conversation
WalkthroughThis pull request updates the secret verification functions in both the client and exporter controllers. The methods Changes
Sequence Diagram(s)Client Secret Check FlowsequenceDiagram
participant CR as ClientReconciler
participant API as Kubernetes API
CR->>CR: Initiate reconcileStatusCredential
CR->>CR: Call clientSecretExists(ctx, client)
alt Credential is nil
CR-->>CR: Return (false, nil)
else Credential exists
CR->>API: Retrieve client secret
API-->>CR: Return (exists, error)
end
CR->>CR: Log error if present
Exporter Secret Check FlowsequenceDiagram
participant ER as ExporterReconciler
participant API as Kubernetes API
ER->>ER: Initiate reconcileStatusCredential
ER->>ER: Call exporterSecretExists(ctx, exporter)
alt Credential is nil
ER-->>ER: Return (false, nil)
else Credential exists
ER->>API: Retrieve exporter secret (ignoring not-found)
API-->>ER: Return (exists, error)
end
ER->>ER: Log error if present
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
internal/controller/client_controller.go (1)
109-109: Make log message style consistent.Consider prefixing the log message with "reconcileStatusCredential:" to maintain consistency with other log messages in the file.
- logger.Info("reconcileStatusCredential: the client secret has ceased to exist, will be recreated", "client", client.Name) + logger.Info("reconcileStatusCredential: client secret has ceased to exist, will be recreated", "client", client.Name)internal/controller/exporter_controller.go (1)
112-116: Make log message style consistent.Consider prefixing the log message with "reconcileStatusCredential:" to maintain consistency with other log messages in the file.
- logger.Info("the exporter secret has ceased to exist, will be recreated", "exporter", exporter.Name) + logger.Info("reconcileStatusCredential: exporter secret has ceased to exist, will be recreated", "exporter", exporter.Name)Also applies to: 121-121
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
internal/controller/client_controller.go(1 hunks)internal/controller/exporter_controller.go(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: e2e-tests
- GitHub Check: deploy-kind
🔇 Additional comments (3)
internal/controller/client_controller.go (2)
77-92: LGTM! Good error handling improvements.The updated function signature with
(bool, error)return type and the nil check forclient.Status.Credentialimprove error handling robustness.
100-104: LGTM! Good error handling and logging.The error handling is robust and the log message provides clear context about the failure.
internal/controller/exporter_controller.go (1)
89-104: LGTM! Consistent with client_controller.go.The changes mirror the improvements made in client_controller.go, maintaining consistency across both controllers.
mangelajo
left a comment
There was a problem hiding this comment.
Thanks X-D, I was coming back and looking to add that suggestion but I see it's merged and proposed
Summary by CodeRabbit