Skip to content

Consolidate Manifest Deployment Steps in Deploy Template Workflow#27582

Merged
RachalCassity merged 5 commits intomasterfrom
rcassity-sandbox-helm-charts
Apr 6, 2026
Merged

Consolidate Manifest Deployment Steps in Deploy Template Workflow#27582
RachalCassity merged 5 commits intomasterfrom
rcassity-sandbox-helm-charts

Conversation

@RachalCassity
Copy link
Copy Markdown
Member

@RachalCassity RachalCassity commented Apr 6, 2026

Keep your PR as a Draft until it's ready for Platform review. A PR is ready for Platform review when it has a teammate approval and tests, linting, and settings checks pass CI. See these tips on how to avoid common delays in getting your PR merged.

Summary

Two separate workflow steps that updated image tags and DataDog version secrets in the manifest repo have been merged into a single unified step.

Previously the workflow used two hardcoded steps — one targeting dev and staging using the vets-api-parent chart key, and a second targeting prod and sandbox using vets-api. This was brittle because the chart key was assumed based on environment name rather than verified against the actual values.yaml.

The new step iterates over all four environments and uses yq's has() function to detect which chart key is present in each environment's values.yaml at runtime, storing the results in variables rather than piping through grep to avoid pipefail interaction issues. If both keys are found in the same file the workflow exits with a non-zero code rather than silently applying the wrong key. If neither key is found the environment is skipped with a warning.

Expected behavior:

  • Removes duplicated yq commands across two steps into a single shared block
  • Eliminates hardcoded environment-to-chart-key assumptions — the correct key is detected from the file itself
  • Fails loudly if both keys are present rather than silently defaulting to one
  • Removes unused ECR_REGISTRY and ECR_REPOSITORY env vars that were never referenced in the run block

Related issue(s)

Testing done

  • New code is covered by unit tests
  • Describe what the old behavior was prior to the change
  • Describe the steps required to verify your changes are working as expected. Exclusively stating 'Specs run' is NOT acceptable as appropriate testing
  • If this work is behind a flipper:
    • Tests need to be written for both the flipper on and flipper off scenarios. Docs.
    • What is the testing plan for rolling out the feature?

Screenshots

Note: Optional

What areas of the site does it impact?

(Describe what parts of the site are impacted andifcode touched other areas)

Acceptance criteria

  • I fixed|updated|added unit tests and integration tests for each feature (if applicable).
  • No error nor warning in the console.
  • Events are being sent to the appropriate logging solution
  • Documentation has been updated (link to documentation)
  • No sensitive information (i.e. PII/credentials/internal URLs/etc.) is captured in logging, hardcoded, or specs
  • Feature/bug has a monitor built into Datadog (if applicable)
  • If app impacted requires authentication, did you login to a local build and verify all authenticated routes work as expected
  • I added a screenshot of the developed feature

Requested Feedback

(OPTIONAL)What should the reviewers know in addition to the above. Is there anything specific you wish the reviewer to assist with. Do you have any concerns with this PR, why?

@RachalCassity RachalCassity requested a review from a team as a code owner April 6, 2026 13:53
Copilot AI review requested due to automatic review settings April 6, 2026 13:53
@RachalCassity RachalCassity changed the title Refactored the image and version deployment in the deploy template wo… Refactored the image and version deployment in the deploy template Apr 6, 2026
@RachalCassity RachalCassity changed the title Refactored the image and version deployment in the deploy template Consolidate Manifest Deployment Steps in Deploy Template Workflow Apr 6, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR refactors the GitHub Actions deploy workflow template to consolidate how the manifests repo is updated with the new image tag and Datadog version/commit values across multiple environments.

Changes:

  • Combine the previous per-environment update loops into a single loop over dev, staging, prod, and sandbox.
  • Add logic to detect whether values.yaml uses vets-api-parent vs vets-api as the chart key and update accordingly.
  • Keep a single git diff output after applying all yq edits.

Comment thread .github/workflows/deploy-template.yml
Comment thread .github/workflows/deploy-template.yml Outdated
Copy link
Copy Markdown
Contributor

@jweissman jweissman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good -- happy to approve but left some comments that might be good to reflect on. The only big-picture suggestion might be incorporating -e flag to set exit code (in which case we could add || exit 1 to bail if something is missing but you're handling that otherwise so maybe fine!)

Comment thread .github/workflows/deploy-template.yml
Comment thread .github/workflows/deploy-template.yml Outdated
jweissman
jweissman previously approved these changes Apr 6, 2026
@RachalCassity RachalCassity enabled auto-merge (squash) April 6, 2026 15:07
@RachalCassity RachalCassity merged commit 2261917 into master Apr 6, 2026
42 of 43 checks passed
@RachalCassity RachalCassity deleted the rcassity-sandbox-helm-charts branch April 6, 2026 15:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants