add build-push-deploy #11

hamishfagg · 2025-08-17T21:27:43Z

No description provided.

entelligence-ai-pr-reviews · 2025-08-17T21:51:04Z

Review Summary

entelligence-ai-pr-reviews · 2025-08-17T22:13:56Z

Review Summary

🏷️ Draft Comments (1)

Skipped posting 1 draft comments that were valid but scored below your review threshold (>=13/15). Feel free to update them here.
build-push-ecr/action.yml (1)
57-57: extra-build-args is passed directly to docker buildx build without sanitization, allowing command injection if attacker controls this input.

📊 Impact Scores:

Production Impact: 4/5

Fix Specificity: 4/5

Urgency Impact: 4/5

Total Score: 12/15

🤖 AI Agent Prompt (Copy & Paste Ready):
In build-push-ecr/action.yml, line 57, the input `extra-build-args` is passed directly to the shell in the `docker buildx build` command, which allows for command injection if an attacker controls this input. Add input validation before this line to ensure only safe characters are allowed (e.g., alphanumerics, dashes, underscores, equals, commas, and periods). If invalid characters are detected, print an error and exit. Do not change the formatting or indentation of the original code.

🔍 Comments beyond diff scope (1)

.github/workflows/build-push-deploy.yml (1)

9-9: inputs.environment-name is referenced in the input description and as a key, but the input is now named stage-name, which will cause runtime input resolution failures.
Category: correctness

.github/workflows/build-push-deploy.yml

entelligence-ai-pr-reviews · 2025-09-15T04:09:01Z

🔒 Entelligence AI Vulnerability Scanner

✅ No security vulnerabilities found!

Your code passed our comprehensive security analysis.

entelligence-ai-pr-reviews · 2025-10-06T20:37:18Z

.github/workflows/build-push-deploy.yml

+        # Do the actual deployment
+        with:
+          environment-slug: ${{matrix.deploy-env}}
+          k8s-namespace: ${{inputs.deploy-namespace || matrix.deploy-env}}


correctness: k8s-namespace assignment uses ${{inputs.deploy-namespace || matrix.deploy-env}}, but GitHub Actions expressions do not support || for defaulting, causing the value to be set to the literal string or empty if deploy-namespace is not provided, resulting in deployments to the wrong namespace or failure.

🤖 AI Agent Prompt for Cursor/Windsurf

📋 Copy this prompt to your AI coding assistant (Cursor, Windsurf, etc.) to get help fixing this issue

In .github/workflows/build-push-deploy.yml, line 123, the expression for `k8s-namespace` uses `${{inputs.deploy-namespace || matrix.deploy-env}}`, but GitHub Actions expressions do not support `||` for defaulting. Update this line to use a supported conditional expression so that if `inputs.deploy-namespace` is set (not empty), it is used; otherwise, fall back to `matrix.deploy-env`. Replace the line with: `k8s-namespace: ${{ inputs.deploy-namespace != '' && inputs.deploy-namespace || matrix.deploy-env }}`.

📝 Committable Code Suggestion

‼️ Ensure you review the code suggestion before committing it to the branch. Make sure it replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change

k8s-namespace: ${{inputs.deploy-namespace || matrix.deploy-env}}

k8s-namespace: ${{ inputs.deploy-namespace != '' && inputs.deploy-namespace || matrix.deploy-env }}

entelligence-ai-pr-reviews · 2025-10-09T01:22:44Z

.github/workflows/build-push-deploy.yml

+
+  migrate:
+    runs-on: mdb-dev
+    container: ${{ secrets.ECR_REGISTRY }}/${{ inputs.service-name }}:${{ inputs.stage-name }}-${{ inputs.docker-image-ref }}


correctness: migrate job's container image reference uses ${{ inputs.docker-image-ref }} which may be undefined if not provided, causing the job to fail to start.

🤖 AI Agent Prompt for Cursor/Windsurf

📋 Copy this prompt to your AI coding assistant (Cursor, Windsurf, etc.) to get help fixing this issue

In .github/workflows/build-push-deploy.yml, line 97, the `migrate` job's `container` field references `${{ inputs.docker-image-ref }}` directly, which may be undefined if not provided, causing the job to fail. Update the container image reference to append `-${{ inputs.docker-image-ref }}` only if `docker-image-ref` is set, otherwise omit it. Ensure the job works whether or not `docker-image-ref` is provided.

📝 Committable Code Suggestion

‼️ Ensure you review the code suggestion before committing it to the branch. Make sure it replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change

container: ${{ secrets.ECR_REGISTRY }}/${{ inputs.service-name }}:${{ inputs.stage-name }}-${{ inputs.docker-image-ref }}

0097 [+]: container: ${{ secrets.ECR_REGISTRY }}/${{ inputs.service-name }}:${{ inputs.stage-name }}${{ inputs.docker-image-ref && format('-{0}', inputs.docker-image-ref) || '' }}

entelligence-ai-pr-reviews · 2025-10-09T01:34:49Z

.github/workflows/build-push-deploy.yml

+    strategy:
+      matrix:
+        deploy-env: ${{fromJson(needs.get-deploy-labels.outputs.deploy-envs)}}
+    container: ${{ vars.ECR_REGISTRY }}/${{ inputs.service-name }}:${{ inputs.stage-name }}-${{ inputs.docker-image-ref }}


correctness: container image reference changed from secrets.ECR_REGISTRY to vars.ECR_REGISTRY, which may break runtime if vars.ECR_REGISTRY is not set or does not match the expected registry value.

🤖 AI Agent Prompt for Cursor/Windsurf

📋 Copy this prompt to your AI coding assistant (Cursor, Windsurf, etc.) to get help fixing this issue

In .github/workflows/build-push-deploy.yml, line 101, the `container` image reference was changed from `secrets.ECR_REGISTRY` to `vars.ECR_REGISTRY`. This may cause runtime failures if `vars.ECR_REGISTRY` is not set or does not match the expected registry. Please revert this line to use `secrets.ECR_REGISTRY` instead.

📝 Committable Code Suggestion

‼️ Ensure you review the code suggestion before committing it to the branch. Make sure it replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change

container: ${{ vars.ECR_REGISTRY }}/${{ inputs.service-name }}:${{ inputs.stage-name }}-${{ inputs.docker-image-ref }}

container: ${{ secrets.ECR_REGISTRY }}/${{ inputs.service-name }}:${{ inputs.stage-name }}-${{ inputs.docker-image-ref }}

entelligence-ai-pr-reviews · 2025-10-09T01:38:26Z

.github/workflows/build-push-deploy.yml

+    container: ${{ vars.ECR_REGISTRY }}/${{ inputs.service-name }}:${{ inputs.stage-name }}-${{ inputs.docker-image-ref }}
+    if: ${{ needs.get-deploy-labels.outputs.deploy-envs != '[]' }}
+    steps:
+      - name: Login to Amazon ECR
+        id: login-ecr
+        uses: aws-actions/amazon-ecr-login@v2


correctness: aws-actions/amazon-ecr-login@v2 step is added after the container image is referenced, which may cause ECR authentication to fail if the image is not already available locally.

🤖 AI Agent Prompt for Cursor/Windsurf

📋 Copy this prompt to your AI coding assistant (Cursor, Windsurf, etc.) to get help fixing this issue

In .github/workflows/build-push-deploy.yml, lines 101-106, the ECR login step is defined after the container image is referenced, which can cause authentication failures if the image is not already present. Move the 'Login to Amazon ECR' step before the 'container:' definition so that authentication occurs before the image is pulled.

📝 Committable Code Suggestion

‼️ Ensure you review the code suggestion before committing it to the branch. Make sure it replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change

container: ${{ vars.ECR_REGISTRY }}/${{ inputs.service-name }}:${{ inputs.stage-name }}-${{ inputs.docker-image-ref }}

if: ${{ needs.get-deploy-labels.outputs.deploy-envs != '[]' }}

steps:

- name: Login to Amazon ECR

id: login-ecr

uses: aws-actions/amazon-ecr-login@v2

if: ${{ needs.get-deploy-labels.outputs.deploy-envs != '[]' }}

steps:

- name: Login to Amazon ECR

id: login-ecr

uses: aws-actions/amazon-ecr-login@v2

container: ${{ vars.ECR_REGISTRY }}/${{ inputs.service-name }}:${{ inputs.stage-name }}-${{ inputs.docker-image-ref }}

entelligence-ai-pr-reviews · 2025-10-09T02:07:51Z

.github/workflows/build-push-deploy.yml

+          export NAMESPACE=${{inputs.deploy-namespace || matrix.deploy-env}}
+          export IMAGE_TAG=${{ inputs.stage-name }}-${{ inputs.docker-image-ref }}
+          export JOB_NAME=$(grep -E '^ *name:' ${{ inputs.migration-job-file }} | head -1 | awk '{print $2}')
+
+          kubectl -n $NAMESPACE delete job --ignore-not-found $JOB_NAME
+          envsubst '${IMAGE_TAG} ${NAMESPACE}' < ${{ inputs.migration-job-file }} | kubectl apply -f -


correctness: envsubst '${IMAGE_TAG} ${NAMESPACE}' < ${{ inputs.migration-job-file }} will not substitute variables in the YAML unless the YAML contains ${IMAGE_TAG} and ${NAMESPACE} placeholders; if the YAML uses a different variable format or hardcoded values, the migration job will use incorrect image or namespace.

🤖 AI Agent Prompt for Cursor/Windsurf

📋 Copy this prompt to your AI coding assistant (Cursor, Windsurf, etc.) to get help fixing this issue

In .github/workflows/build-push-deploy.yml, lines 111-116, the workflow uses `envsubst '${IMAGE_TAG} ${NAMESPACE}' < ${{ inputs.migration-job-file }}` to substitute variables in the migration job YAML. This only works if the YAML contains exactly `${IMAGE_TAG}` and `${NAMESPACE}` placeholders. If the YAML uses different variable names or hardcoded values, the migration job will not use the correct image or namespace, leading to failed or incorrect migrations. Change the line to use `envsubst < ${{ inputs.migration-job-file }}` so all environment variables are substituted, ensuring the correct values are injected.

📝 Committable Code Suggestion

‼️ Ensure you review the code suggestion before committing it to the branch. Make sure it replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change

export NAMESPACE=${{inputs.deploy-namespace || matrix.deploy-env}}

export IMAGE_TAG=${{ inputs.stage-name }}-${{ inputs.docker-image-ref }}

export JOB_NAME=$(grep -E '^ *name:' ${{ inputs.migration-job-file }} | head -1 | awk '{print $2}')

kubectl -n $NAMESPACE delete job --ignore-not-found $JOB_NAME

envsubst '${IMAGE_TAG} ${NAMESPACE}' < ${{ inputs.migration-job-file }} | kubectl apply -f -

export NAMESPACE=${{inputs.deploy-namespace || matrix.deploy-env}}

export IMAGE_TAG=${{ inputs.stage-name }}-${{ inputs.docker-image-ref }}

export JOB_NAME=$(grep -E '^ *name:' ${{ inputs.migration-job-file }} | head -1 | awk '{print $2}')

kubectl -n $NAMESPACE delete job --ignore-not-found $JOB_NAME

envsubst < ${{ inputs.migration-job-file }} | kubectl apply -f -

entelligence-ai-pr-reviews · 2025-10-09T02:07:53Z

.github/workflows/build-push-deploy.yml

+          kubectl -n $NAMESPACE delete job --ignore-not-found $JOB_NAME
+          envsubst '${IMAGE_TAG} ${NAMESPACE}' < ${{ inputs.migration-job-file }} | kubectl apply -f -
+
+          kubectl -n "$NAMESPACE" wait --for=condition=complete --timeout=1m "job/$JOB_NAME"


performance: kubectl -n "$NAMESPACE" wait --for=condition=complete --timeout=1m "job/$JOB_NAME" uses a fixed 1-minute timeout, which can cause unnecessary resource contention or failed deployments for large/slow migrations, impacting reliability and user experience.

🤖 AI Agent Prompt for Cursor/Windsurf

📋 Copy this prompt to your AI coding assistant (Cursor, Windsurf, etc.) to get help fixing this issue

In .github/workflows/build-push-deploy.yml, line 118, the migration job wait step uses a hardcoded 1-minute timeout, which can cause failures for long-running migrations and resource contention. Refactor this to use a configurable timeout (e.g., an input like `migration-wait-timeout` with a sensible default such as 5m), so that deployments can scale for larger jobs.

📝 Committable Code Suggestion

‼️ Ensure you review the code suggestion before committing it to the branch. Make sure it replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change

kubectl -n "$NAMESPACE" wait --for=condition=complete --timeout=1m "job/$JOB_NAME"

kubectl -n "$NAMESPACE" wait --for=condition=complete --timeout=${{ inputs.migration-wait-timeout || '5m' }} "job/$JOB_NAME"

hamishfagg added 12 commits August 18, 2025 09:21

initial

8f30a64

fix

63df8dd

fix

b0f0f27

fix

d2736ba

fix

10bfe9b

fix

d89cc67

fix

7ae5c59

fix

8facb60

fix

458c9f0

fix

bfd5d44

fix

7e9a827

fix

8e45f86

hamishfagg added 2 commits August 18, 2025 09:53

fix

f97bb17

fix

b5a22fd

entelligence-ai-pr-reviews bot reviewed Aug 17, 2025

View reviewed changes

.github/workflows/build-push-deploy.yml Show resolved Hide resolved

hamishfagg added 6 commits August 18, 2025 10:17

fix

b855cba

fix

f20315e

fix

318229d

fix

c58b909

fix

e0786b1

fix

dc832e8

mindsdb-devops approved these changes Aug 18, 2025

View reviewed changes

hamishfagg added 2 commits August 25, 2025 17:59

WIP

7eb5d87

add bake

2769466

add deploy namespace

0594a3a

entelligence-ai-pr-reviews bot reviewed Oct 6, 2025

View reviewed changes

add migration

5aef156

entelligence-ai-pr-reviews bot reviewed Oct 9, 2025

View reviewed changes

secret->var

c7fd0f7

entelligence-ai-pr-reviews bot reviewed Oct 9, 2025

View reviewed changes

log into ecr

3b20427

entelligence-ai-pr-reviews bot reviewed Oct 9, 2025

View reviewed changes

fix migrate

833709f

entelligence-ai-pr-reviews bot reviewed Oct 9, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

add build-push-deploy #11

add build-push-deploy #11

Uh oh!

hamishfagg commented Aug 17, 2025

Uh oh!

entelligence-ai-pr-reviews bot commented Aug 17, 2025

Uh oh!

entelligence-ai-pr-reviews bot commented Aug 17, 2025

Uh oh!

Uh oh!

entelligence-ai-pr-reviews bot commented Sep 15, 2025

Uh oh!

entelligence-ai-pr-reviews bot Oct 6, 2025

Uh oh!

entelligence-ai-pr-reviews bot Oct 9, 2025

Uh oh!

entelligence-ai-pr-reviews bot Oct 9, 2025

Uh oh!

entelligence-ai-pr-reviews bot Oct 9, 2025

Uh oh!

entelligence-ai-pr-reviews bot Oct 9, 2025

Uh oh!

entelligence-ai-pr-reviews bot Oct 9, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	k8s-namespace: ${{inputs.deploy-namespace \|\| matrix.deploy-env}}
	k8s-namespace: ${{ inputs.deploy-namespace != '' && inputs.deploy-namespace \|\| matrix.deploy-env }}

	container: ${{ secrets.ECR_REGISTRY }}/${{ inputs.service-name }}:${{ inputs.stage-name }}-${{ inputs.docker-image-ref }}
	0097 [+]: container: ${{ secrets.ECR_REGISTRY }}/${{ inputs.service-name }}:${{ inputs.stage-name }}${{ inputs.docker-image-ref && format('-{0}', inputs.docker-image-ref) \|\| '' }}

	container: ${{ vars.ECR_REGISTRY }}/${{ inputs.service-name }}:${{ inputs.stage-name }}-${{ inputs.docker-image-ref }}
	container: ${{ secrets.ECR_REGISTRY }}/${{ inputs.service-name }}:${{ inputs.stage-name }}-${{ inputs.docker-image-ref }}

	kubectl -n "$NAMESPACE" wait --for=condition=complete --timeout=1m "job/$JOB_NAME"
	kubectl -n "$NAMESPACE" wait --for=condition=complete --timeout=${{ inputs.migration-wait-timeout \|\| '5m' }} "job/$JOB_NAME"

add build-push-deploy #11

Are you sure you want to change the base?

add build-push-deploy #11

Uh oh!

Conversation

hamishfagg commented Aug 17, 2025

Uh oh!

entelligence-ai-pr-reviews bot commented Aug 17, 2025

Review Summary

Uh oh!

entelligence-ai-pr-reviews bot commented Aug 17, 2025

Review Summary

Uh oh!

Uh oh!

entelligence-ai-pr-reviews bot commented Sep 15, 2025

Uh oh!

entelligence-ai-pr-reviews bot Oct 6, 2025

Choose a reason for hiding this comment

Uh oh!

entelligence-ai-pr-reviews bot Oct 9, 2025

Choose a reason for hiding this comment

Uh oh!

entelligence-ai-pr-reviews bot Oct 9, 2025

Choose a reason for hiding this comment

Uh oh!

entelligence-ai-pr-reviews bot Oct 9, 2025

Choose a reason for hiding this comment

Uh oh!

entelligence-ai-pr-reviews bot Oct 9, 2025

Choose a reason for hiding this comment

Uh oh!

entelligence-ai-pr-reviews bot Oct 9, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants