Skip to content
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

Fix daily semconv build workflow #6034

Merged
merged 2 commits into from
Jan 23, 2025
Merged

Conversation

trask
Copy link
Member

@trask trask commented Jan 22, 2025

Secrets are not automatically passed to reusable workflows

cc @open-telemetry/specs-semconv-maintainers

@trask trask requested a review from a team as a code owner January 22, 2025 23:18
Copy link
Contributor

@chalin chalin left a comment

Choose a reason for hiding this comment

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

If ever workflow arguments are visible (e.g., logged), is GH smart enough to not show the value when it is meant to be a secret?

@trask
Copy link
Member Author

trask commented Jan 23, 2025

If ever workflow arguments are visible (e.g., logged), is GH smart enough to not show the value when it is meant to be a secret?

I tried to echo $GITHUB_TOKEN recently, and GH thwarted me, but I wouldn't trust it completely, do you have a specific concern about this workflow?

@chalin
Copy link
Contributor

chalin commented Jan 23, 2025

do you have a specific concern about this workflow?

That secrets.OPENTELEMETRYBOT_GITHUB_TOKEN is being passed as an argument value to a workflow, so my concern was to question whether that puts us at risk of exposing the token via GH workflow run logs. WDYT?

@trask
Copy link
Member Author

trask commented Jan 23, 2025

it's not passed as a regular input, instead it's explicitly passed as a secret. I believe that's the reason for having a separate way to pass secrets that's different from passing regular inputs

Copy link
Contributor

@chalin chalin left a comment

Choose a reason for hiding this comment

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

Right, of course, now that I'm paying closer attention. Ok, LGTM.

Btw, we might want to see how we can tweak this so that it doesn't fail if the refcache is updated, only if there are broken links. That might be a project for another time.

@chalin chalin added this pull request to the merge queue Jan 23, 2025
Merged via the queue into open-telemetry:main with commit 5ee3c7e Jan 23, 2025
17 checks passed
@trask trask deleted the fix-workflow branch January 29, 2025 03:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants