Skip to content

DCMAW-11654: investigating github actions#451

Draft
kikidawson-gds wants to merge 4 commits into
mainfrom
DCMAW-11654
Draft

DCMAW-11654: investigating github actions#451
kikidawson-gds wants to merge 4 commits into
mainfrom
DCMAW-11654

Conversation

@kikidawson-gds
Copy link
Copy Markdown
Contributor

@kikidawson-gds kikidawson-gds commented Mar 3, 2025

DCMAW-11654

What changed

This is an investigation into standardising github action workflows.

  • Decided against using matrices for environments because it didn't decrease lines of code enough for the complexity added
  • CI-check workflow doesn't run when PR is in draft. Workflows can be manually run if devs wish to confirm changes before PR is ready for review
  • sam validate --lint is to be used over cfn-lint because it's already included in the ubuntu version. We can still provide custom rules for cfn-lint in config file when using sam.
  • ubuntu24.04 is being used instead of 22.04 as it's smaller and doesn't include so many unnecessary packages
  • reusable workflows have been used over composite actions as they allow devs to see each step when running the pipelines. Composite actions would condense all the steps into one with less visibility with errors.

Why did it change

Checklists

  • There is a ticket raised for this PR that is present in the branch name
  • No PII data logged. See guidance here
  • Demo to a BA, TA, and the team.
  • Update README with any new instructions or tasks

@kikidawson-gds kikidawson-gds force-pushed the DCMAW-11654 branch 3 times, most recently from 9136f86 to 20edfed Compare March 3, 2025 11:53
Comment thread .github/workflows/backend-api-post-merge.yml
Comment thread .github/workflows/backend-api-post-merge.yml
aws-region: eu-west-2
role-to-assume: ${{ secrets.GH_ACTIONS_ROLE_ARN }}

# - name: Cache SAM builds
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

DCMAW-11675: think about caching

Comment thread .github/workflows/job_ci-checks.yml Outdated

- name: Run unit, infra, and pact tests
run: npm run test
# output the file the unit tests generates for use in sonar quality gate
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

DCMAW-11675: Can we output the results from the unit tests for use in the sonar checks

Comment thread .github/workflows/job_sonarqube.yml

dev-post-merge:
name: Dev Post Merge
needs: sonarqube-scan
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

DCMAW-11675: Does this need dependence on sonar?

@@ -0,0 +1,60 @@
name: Post Merge
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Not completely convinced we need this middleman workflow but I like how simple it makes the backend-api-post-merge and test-resources-post-merge yamls

@kikidawson-gds kikidawson-gds force-pushed the DCMAW-11654 branch 2 times, most recently from 3eaf0ae to 4d22d78 Compare March 3, 2025 12:52
Comment thread .github/workflows/job_ci-checks.yml Outdated
run: npm run test
# output the file the unit tests generates for use in sonar quality gate

- name: Generate proxy open api spec
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Do we need this?

Comment thread .github/workflows/job_ci-checks.yml
@kikidawson-gds kikidawson-gds force-pushed the DCMAW-11654 branch 12 times, most recently from 957fd49 to c49f03d Compare March 3, 2025 16:44
Comment thread .github/workflows/job_build-and-push-test-image.yml
Comment thread .github/workflows/job_ci-checks.yml Outdated

# - name: Generate proxy open api spec
# if: ${{ inputs.GENERATE_PROXY_OPEN_API_SPEC }}
# run: npm run generate-proxy-open-api
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

is this needed?

Comment thread .github/workflows/job_ci-checks.yml Outdated
- name: Set up Homebrew
if: ${{ inputs.VERIFY_TEMPLATE_RAIN }}
id: set-up-homebrew
uses: Homebrew/actions/setup-homebrew@a7a36215df86859f163fbb774ebe0cecf9ec8547
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

https://github.com/actions/runner-images/blob/main/images/ubuntu/Ubuntu2404-Readme.md#homebrew-note

https://github.com/govuk-one-login/mobile-id-check-async-infra/blob/main/.github/workflows/pull-request-workflow.yml#L44

Brew is already present and the suggested method of using it for these runners is to update path.

Unfortunately the command suggested in the docs doesn't persist across steps. You could run it on every step that uses brew, or uses a program installed by brew (since the changes in path are required to find the installed programs). In the changes to async-infra i found out what the suggested script does and manually updated the path through GITHUB_ENV to persist across steps.

@kikidawson-gds kikidawson-gds force-pushed the DCMAW-11654 branch 4 times, most recently from ff5b95a to 4a6e73f Compare March 5, 2025 17:02
@kikidawson-gds kikidawson-gds force-pushed the DCMAW-11654 branch 9 times, most recently from a9466a3 to 2c1ccc5 Compare March 6, 2025 17:34
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.

2 participants