Skip to content
This repository was archived by the owner on May 27, 2026. It is now read-only.

fix: gha workflows#16

Merged
mrjoelkamp merged 10 commits into
mainfrom
fix-gha-workflows
Jun 11, 2024
Merged

fix: gha workflows#16
mrjoelkamp merged 10 commits into
mainfrom
fix-gha-workflows

Conversation

@mrjoelkamp
Copy link
Copy Markdown
Contributor

@mrjoelkamp mrjoelkamp commented Jun 10, 2024

Summary

  • removes custom CodeQL workflow in favor of GitHub Advanced Security CodeQL scanning via org settings
  • fixes linter
  • fixes e2e tests

@mrjoelkamp mrjoelkamp force-pushed the fix-gha-workflows branch 2 times, most recently from 57d87dd to e8715a8 Compare June 10, 2024 19:37
@mrjoelkamp mrjoelkamp force-pushed the fix-gha-workflows branch 2 times, most recently from d1edbeb to 3b0667d Compare June 10, 2024 20:30
@mrjoelkamp mrjoelkamp marked this pull request as ready for review June 10, 2024 20:46
@mrjoelkamp mrjoelkamp requested a review from a team as a code owner June 10, 2024 20:46
Comment thread test/bats/test.bats
wait_for_process ${WAIT_TIME} ${SLEEP_TIME} "kubectl -n ${GATEKEEPER_NAMESPACE} wait --for=condition=Ready --timeout=60s pod -l run=external-data-provider"
@test "attest-provider is running" {
wait_for_process ${WAIT_TIME} ${SLEEP_TIME} "kubectl -n ${GATEKEEPER_NAMESPACE} wait --for=condition=Ready --timeout=60s pod -l run=attest-provider"
sleep 5 # we need a readiness probe https://kubernetes.io/docs/tasks/configure-pod-container/configure-liveness-readiness-startup-probes/#define-readiness-probes
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.

I added sleep 5 because the attest provider app isn't ready to accept connections immediately after starting, we need to add a readiness probe to the application so that we avoid error: connection refused responses

Copy link
Copy Markdown

@whalelines whalelines 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, a couple questions

Comment on lines +47 to +48
app-id: ${{ vars.DOCKER_READ_APP_ID }}
private-key: ${{ secrets.DOCKER_READ_APP_PRIVATE_KEY }}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Where do these come from?

Comment thread test/bats/test.bats
Comment on lines -31 to +32
run kubectl apply -f validation/external-data-provider-constraint.yaml
run kubectl apply -f validation/attest-constraint.yaml
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Did this file get renamed in a previous PR?

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.

yes

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.

Copy link
Copy Markdown
Contributor

@jonnystoten jonnystoten left a comment

Choose a reason for hiding this comment

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

LGTM

@mrjoelkamp mrjoelkamp merged commit 861ccb8 into main Jun 11, 2024
@mrjoelkamp mrjoelkamp deleted the fix-gha-workflows branch June 11, 2024 13:24
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants