Generate controller/router secrets with oneshot job #86
Conversation
WalkthroughThis pull request updates the Helm chart and deployment process for the jumpstarter application. It simplifies the secret management by removing specific secret parameters and associated Kubernetes Secret templates and adjusting the linting command in the Makefile. A new job is added to dynamically create the required secrets if they are absent. Additionally, the retry logic in the Helm deployment script has been modified to allow more attempts for gRPC endpoint readiness. Changes
Sequence Diagram(s)sequenceDiagram
participant Job as Jumpstarter Secrets Job
participant API as Kubernetes API
Job->>API: Check if "jumpstarter-router-secret" exists
Job->>API: Check if "jumpstarter-controller-secret" exists
alt Secrets exist
API-->>Job: Return secret details
else Secrets missing
API-->>Job: Indicate secret not found
Job->>API: Create missing secret(s)
API-->>Job: Confirm secret creation
end
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms (4)
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/secrets-job.yaml (1)
1-31: 🛠️ Refactor suggestionAdd resource constraints and security context to the Job.
While the secret generation logic is secure, the Job specification should be hardened with:
- Resource limits to prevent resource exhaustion
- Security context to run with minimal privileges
- Job completion timeout to prevent hanging jobs
Apply this diff to improve the Job specification:
spec: template: metadata: name: jumpstarter-secrets spec: serviceAccountName: controller-manager + securityContext: + runAsNonRoot: true + seccompProfile: + type: RuntimeDefault containers: - name: jumpstarter-secrets image: quay.io/openshift/origin-cli + resources: + limits: + cpu: 200m + memory: 256Mi + requests: + cpu: 100m + memory: 128Mi + securityContext: + allowPrivilegeEscalation: false + capabilities: + drop: + - ALL command: - /bin/sh - -c - | set -e {{- range $name := tuple "jumpstarter-router-secret" "jumpstarter-controller-secret" }} if ! oc get secret {{ $name }} -n {{ $namespace }} >/dev/null 2>&1; then oc create secret generic {{ $name }} -n={{ $namespace }} \ --from-literal=key="$(openssl rand -hex 32)" fi {{- end }} restartPolicy: Never + backoffLimit: 3 + activeDeadlineSeconds: 300🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
🧹 Nitpick comments (2)
hack/deploy_with_helm.sh (1)
110-110: Update error message to match the new retry count.The error message still mentions "60s" but with 60 retries and 2s sleep, the actual wait time is 120s.
Apply this diff to fix the error message:
- echo -e "${GREEN} * ${ep} not ready after 60s, exiting ... ${NC}" + echo -e "${GREEN} * ${ep} not ready after 120s, exiting ... ${NC}"Also applies to: 116-116
Makefile (1)
160-162: Consolidate duplicate helm lint targets.The Makefile has two similar targets for linting Helm charts:
lint-helmandhelm-lint. This could be confusing for developers.Consider consolidating these targets into one. Apply this diff:
.PHONY: lint-helm lint-helm: helm lint deploy/helm/jumpstarter - - -.PHONY: helm-lint -helm-lint: - helm lint ./deploy/helm/jumpstarter/Then update any CI/CD pipelines or documentation that might be using the
helm-linttarget.Also applies to: 218-220
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
Makefile(1 hunks)deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/controller-secret.yaml(0 hunks)deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/router-secret.yaml(0 hunks)deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/secrets-job.yaml(1 hunks)deploy/helm/jumpstarter/charts/jumpstarter-controller/values.yaml(0 hunks)deploy/helm/jumpstarter/values.yaml(0 hunks)hack/deploy_with_helm.sh(1 hunks)
💤 Files with no reviewable changes (4)
- deploy/helm/jumpstarter/charts/jumpstarter-controller/values.yaml
- deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/router-secret.yaml
- deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/controller-secret.yaml
- deploy/helm/jumpstarter/values.yaml
🧰 Additional context used
🪛 YAMLlint (1.35.1)
deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/secrets-job.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: lint-go
- GitHub Check: tests
- GitHub Check: e2e-tests
- GitHub Check: deploy-kind
|
Thank you, this is great, will also improve the interface with argocd :) |
2 participants
Summary by CodeRabbit
New Features
Chores