Conversation
Allow setting an alternate image via variables to "make deploy". For example: IMAGE_PULL_SECRET_PATH="/path/to/pull/secret.yaml" SQL_IMAGE="registry.redhat.io/rhel8/postgresql-12" SQL_VERSION="1-181"
WalkthroughThe pull request introduces enhancements to the Helm deployment configuration for the FlightCTL project. The changes focus on improving image pull flexibility by adding support for a custom image pull secret, updating deployment scripts, and providing more configurable options for database image deployment. The modifications span across Helm template files, values configuration, and deployment scripts, aiming to provide more granular control over container image management. Changes
Poem
🪧 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
🧹 Nitpick comments (4)
deploy/helm/flightctl/templates/flightctl-db-deployment.yaml (1)
20-23: Fix indentation in the imagePullSecrets blockThe indentation of the imagePullSecrets block should be consistent with other spec fields.
Apply this diff to fix the indentation:
{{- if .Values.global.imagePullSecretName }} - imagePullSecrets: - - name: {{ .Values.global.imagePullSecretName }} + imagePullSecrets: + - name: {{ .Values.global.imagePullSecretName }} {{- end }}🧰 Tools
🪛 yamllint (1.35.1)
[warning] 21-21: wrong indentation: expected 4 but found 6
(indentation)
[error] 20-20: syntax error: expected the node content, but found '-'
(syntax)
deploy/helm/flightctl/values.yaml (1)
34-34: Add documentation for the imagePullSecretName parameterThe new parameter should be documented following the existing pattern:
Add this documentation above the parameter:
+## @param global.imagePullSecretName Name of the image pull secret for accessing private registries imagePullSecretName: ""test/scripts/deploy_with_helm.sh (2)
7-10: Enhance variable documentation and validationThe new variables should be better documented and validated:
Apply this diff to improve the documentation and validation:
-# If using images from a private registry, specify a path to a Kubernetes Secret yaml for your pull secret (in the flightctl-internal namespace) -# IMAGE_PULL_SECRET_PATH= -SQL_VERSION=${SQL_VERSION:-"latest"} -SQL_IMAGE=${SQL_IMAGE:-"quay.io/sclorg/postgresql-12-c8s"} +# Configuration for PostgreSQL database +# IMAGE_PULL_SECRET_PATH - Path to Kubernetes Secret yaml for private registry authentication (must be in flightctl-internal namespace) +# SQL_IMAGE - PostgreSQL image to use (default: quay.io/sclorg/postgresql-12-c8s) +# SQL_VERSION - PostgreSQL image version tag (default: latest) + +: "${SQL_VERSION:=latest}" +: "${SQL_IMAGE:=quay.io/sclorg/postgresql-12-c8s}"
70-80: Enhance pull secret validationThe pull secret validation could be more robust:
Apply this diff to improve the validation:
if [ ! -z "$IMAGE_PULL_SECRET_PATH" ]; then + if [ ! -f "$IMAGE_PULL_SECRET_PATH" ]; then + echo "Error: IMAGE_PULL_SECRET_PATH file does not exist: $IMAGE_PULL_SECRET_PATH" + exit 1 + fi + PULL_SECRET_NAME=$(cat "$IMAGE_PULL_SECRET_PATH" | yq .metadata.name) PULL_SECRET_NAMESPACE=$(cat "$IMAGE_PULL_SECRET_PATH" | yq .metadata.namespace) + if [ -z "$PULL_SECRET_NAME" ] || [ -z "$PULL_SECRET_NAMESPACE" ]; then + echo "Error: Invalid secret yaml: missing name or namespace" + exit 1 + fi + if [ "$PULL_SECRET_NAMESPACE" != "flightctl-internal" ]; then echo "Namespace for IMAGE_PULL_SECRET_PATH must be flightctl-internal" exit 1 fi SQL_ARG="$SQL_ARG --set global.imagePullSecretName=${PULL_SECRET_NAME}" kubectl apply -f "$IMAGE_PULL_SECRET_PATH" fi
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
deploy/helm/flightctl/templates/flightctl-db-deployment.yaml(1 hunks)deploy/helm/flightctl/values.yaml(1 hunks)test/scripts/deploy_with_helm.sh(5 hunks)
🧰 Additional context used
🪛 yamllint (1.35.1)
deploy/helm/flightctl/templates/flightctl-db-deployment.yaml
[warning] 21-21: wrong indentation: expected 4 but found 6
(indentation)
[error] 20-20: syntax error: expected the node content, but found '-'
(syntax)
🔇 Additional comments (3)
deploy/helm/flightctl/templates/flightctl-db-deployment.yaml (1)
20-23: LGTM on imagePullSecrets implementation
The conditional block for imagePullSecrets is well implemented:
- Only adds the secret when
.Values.global.imagePullSecretNameis defined - Correctly placed in the pod spec
- Properly templated to use the configured secret name
🧰 Tools
🪛 yamllint (1.35.1)
[warning] 21-21: wrong indentation: expected 4 but found 6
(indentation)
[error] 20-20: syntax error: expected the node content, but found '-'
(syntax)
deploy/helm/flightctl/values.yaml (1)
Line range hint 67-69: Review PostgreSQL image configuration
A few concerns about the database image configuration:
- PostgreSQL 12 is approaching end of life (EOL: November 14, 2024)
- Using "latest" tag is not recommended for production as it makes deployments unpredictable
Consider:
- Upgrading to a newer PostgreSQL version (e.g., 15 or 16)
- Specifying a fixed version tag
Run this script to check available PostgreSQL versions:
test/scripts/deploy_with_helm.sh (1)
49-51: LGTM on Helm arguments construction
The construction of Helm arguments for images is well implemented:
- Correctly uses the new variables
- Maintains consistency across different components
- Properly concatenates multiple settings
2 participants
Allow setting an alternate image via variables to "make deploy". For example:
IMAGE_PULL_SECRET_PATH="/path/to/pull/secret.yaml" SQL_IMAGE="registry.redhat.io/rhel8/postgresql-12" SQL_VERSION="1-181"
Summary by CodeRabbit
New Features
Bug Fixes
Chores