-
Notifications
You must be signed in to change notification settings - Fork 198
Reduce the scope of the legacy gcp secrets #10734
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Reduce the scope of the legacy gcp secrets #10734
Conversation
|
This pull request does not have a backport label. Could you fix it @fr4nc1sc0-r4m0n? 🙏
|
.buildkite/hooks/pre-command
Outdated
| fi | ||
|
|
||
| if [[ "$BUILDKITE_STEP_KEY" == *"integration-tests"* ]]; then | ||
| if [[ "$BUILDKITE_PIPELINE_SLUG" == "integration-test-matrix" ]]; then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you confirm if the name of the pipeline is correct? See
elastic-agent/catalog-info.yaml
Lines 360 to 371 in 5546bd7
| spec: | |
| type: buildkite-pipeline | |
| owner: group:ingest-fp | |
| system: platform-ingest | |
| implementation: | |
| apiVersion: buildkite.elastic.dev/v1 | |
| kind: Pipeline | |
| metadata: | |
| name: buildkite-elastic-agent-integration-matrix | |
| description: Runs elastic-agent integration tests for all supported platforms | |
| spec: | |
| pipeline_file: ".buildkite/pipeline.integration-test-matrix.yml" |
I can see BUILDKITE_PIPELINE_SLUG="buildkite-elastic-agent-integration-matrix" in the builds:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are absolutely right! Thanks
| fi | ||
|
|
||
| if [[ "$BUILDKITE_STEP_KEY" == *"integration-tests"* ]]; then | ||
| if [[ "$BUILDKITE_PIPELINE_SLUG" == "buildkite-elastic-agent-integration-matrix" ]]; then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IIUC, API_KEY_TOKEN is used in the its, so we will need to create a new conditional for the matrix and the GCP
.buildkite/hooks/pre-exit
Outdated
| if [ -n "$GOOGLE_APPLICATION_CREDENTIALS" ]; then | ||
| if test -f "$GOOGLE_APPLICATION_CREDENTIALS"; then | ||
| rm $GOOGLE_APPLICATION_CREDENTIALS | ||
| if [[ "$BUILDKITE_PIPELINE_SLUG" == "buildkite-elastic-agent-integration-matrix" ]]; then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we don't need this conditional here, I think the conditionals are safe here:
- if variable exists
- if file exists
Regarldess of the BK pipeline, let's keep it simple
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought the same but found this error:
The thing is that I don't see the set -u option set which is the reason for this error so I preferred to check the pipeline slug as we did in the pre-command.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change:
"$GOOGLE_APPLICATION_CREDENTIALS"→"${GOOGLE_APPLICATION_CREDENTIALS:-}""$TEST_INTEG_AUTH_GCP_SERVICE_TOKEN_FILE"→"${TEST_INTEG_AUTH_GCP_SERVICE_TOKEN_FILE:-}"
This ensures the pre-exit won't fail when these environment variables are not set and set -u is set.
💛 Build succeeded, but was flaky
Failed CI StepsHistory
|
What does this PR do?
This PR aims to reduce the scope of the legacy
kv/ci-shared/observability-ingest/cloud/gcpsecret only for theintegration-test-matrixpipeline.This is part of the global process about removing this secret usage.
Why is it important?
It allows us to simplify the pre-command script and to limit the usage of a legacy secret which will be removed in the future.
Checklist
./changelog/fragmentsusing the changelog toolDisruptive User Impact
How to test this PR locally
Related issues
Questions to ask yourself