-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
replace api_key with gcp secret #15080
Conversation
⏱️ 7h 24m total CI duration on this PR
🚨 1 job on the last run was significantly faster/slower than expected
|
62b78e9
to
8d2d6bd
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
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.
There's multiple of the example .yaml files. Can we clean them up so there's only 1?
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.
Whenever we import new transactions for the testing framework, do we need to update this file with the versions/filename? I haven't been updating this file bc I thought it was used as an example yaml file.
If so, maybe we can update the folder to something else instead of example_tests
, maybe like imported_transactions
?
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.
yes we should renamed this folder, example_tests is a very tricky name lol
…ary example config yaml
8d2d6bd
to
6543923
Compare
sed -i "s/TESTNET_API_KEY/${{ steps.api_key_tokens.outputs.testnet_api_key }}/g" ./imported_transactions/imported_transactions.yaml | ||
sed -i "s/MAINNET_API_KEY/${{ steps.api_key_tokens.outputs.mainnet_api_key }}/g" ./imported_transactions/imported_transactions.yaml | ||
|
||
cat ./imported_transactions/imported_transactions.yaml # Print the updated file for verification |
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.
The cat
command here prints API keys to public workflow logs, creating a security risk. While the sed
commands confirm the file modifications are working, exposing sensitive credentials should be avoided. Consider removing this debug line or redirecting the output to a secure location if verification is needed.
Spotted by Graphite Reviewer
Is this helpful? React 👍 or 👎 to let us know.
9c86e8b
to
f429c53
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
f429c53
to
d4d7f74
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
✅ Forge suite
|
✅ Forge suite
|
✅ Forge suite
|
Description
This is to fix trigger processor tests workflow where it's failing to generate files due to having no api keys specified in the yuaml file. we have added a new secret to GCP and this change updates the workflow to pull the tokens from GCP and replace the placeholder string with the tokens.
How Has This Been Tested?
https://github.com/aptos-labs/aptos-core/actions/runs/11525821798/job/32088828463
Key Areas to Review
Type of Change
Which Components or Systems Does This Change Impact?
Checklist