Skip to content

PB-851 part 1: use Buildkite Secret in favor of K8s Secret#754

Merged
zhming0 merged 1 commit into
mainfrom
ming/pb-851-part-1
Oct 24, 2025
Merged

PB-851 part 1: use Buildkite Secret in favor of K8s Secret#754
zhming0 merged 1 commit into
mainfrom
ming/pb-851-part-1

Conversation

@zhming0
Copy link
Copy Markdown
Contributor

@zhming0 zhming0 commented Oct 23, 2025

This PR replaces all k8s secret reference to Buildkite secrets with proper access policy.

Note, because current Buildkite Secret restrict BUILDKITE_** env var names, I had to rename some env var names.

@zhming0 zhming0 force-pushed the ming/pb-851-part-1 branch 4 times, most recently from 7cd29d5 to bd40b42 Compare October 23, 2025 23:21
@zhming0 zhming0 marked this pull request as ready for review October 23, 2025 23:32
@zhming0 zhming0 requested a review from a team as a code owner October 23, 2025 23:32
@zhming0 zhming0 requested review from a team and swaller-bk October 23, 2025 23:32
Comment thread .buildkite/steps/deploy.sh Outdated
Comment thread cmd/controller/controller_test.go Outdated
Comment on lines 162 to 164
// The buildkite token is required, but it is set from a Kubernetes secret, not the config file,
// which is itself set from a config map that is used to create env variables in the controller
// container. As this is required, we set it here to avoid the validation error.
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.

This comment is a lie now, right? It's set from a buildkite secret during the deploy process

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.

This comment is confusing, I removed it and reworked this bit of code.

Comment thread cmd/controller/flags.go
"name of the Buildkite agent token secret",
)
cmd.Flags().String("buildkite-token", "", "Deprecated - Buildkite API token with GraphQL scopes")
cmd.Flags().String("integration-test-buildkite-token", "", "Deprecated - Buildkite API token with GraphQL scopes")
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.

i think now that we're only using the graphQL token in tests, we should remove it from the stack entirely and pass it to the tests by environment variable or similar — its presence in the stack configuration is only going to confuse people

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.

Totally agree, but that's probably out-of-scope for this PR.

Comment thread .buildkite/steps/deploy.sh Outdated
@zhming0 zhming0 force-pushed the ming/pb-851-part-1 branch from bd40b42 to 1953d11 Compare October 24, 2025 00:45
@zhming0 zhming0 force-pushed the ming/pb-851-part-1 branch from 1953d11 to dcf870a Compare October 24, 2025 00:48
@zhming0 zhming0 requested review from moskyb and swaller-bk October 24, 2025 00:50
@zhming0 zhming0 merged commit 67d8824 into main Oct 24, 2025
1 check passed
@zhming0 zhming0 deleted the ming/pb-851-part-1 branch October 24, 2025 03:10
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