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

Switch to PAT generated GH token #773

Merged
merged 3 commits into from
Jul 23, 2024

Conversation

ev1yehor
Copy link
Contributor

@ev1yehor ev1yehor commented Jul 2, 2024

What does this PR do?

Switch to PAT generated GH token

Why is it important?

Checklist

Related issues

@ev1yehor ev1yehor added the ci label Jul 2, 2024
@ev1yehor ev1yehor self-assigned this Jul 2, 2024
@ev1yehor ev1yehor requested a review from a team as a code owner July 2, 2024 11:02
@ev1yehor ev1yehor marked this pull request as draft July 2, 2024 11:57
Comment on lines +14 to +17
GITHUB_USERNAME_SECRET="elasticmachine"
export GITHUB_USERNAME_SECRET=$GITHUB_USERNAME_SECRET
export GITHUB_EMAIL_SECRET="elasticmachine@elastic.co"
export GITHUB_TOKEN=$VAULT_GITHUB_TOKEN
Copy link
Contributor

@mrodm mrodm Jul 12, 2024

Choose a reason for hiding this comment

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

If ephemeral tokens are used, maybe it's not needed those environment variables.

Are those ephemeral tokens already available for git commands ?

If they are, we could try to remove the usage of these env. vars in these functions and test it:

set_git_config() {
git config user.name "${GITHUB_USERNAME_SECRET}"
git config user.email "${GITHUB_EMAIL_SECRET}"
}
git_push_with_auth() {
local owner="$1"
local repository="$2"
local branch="$3"
retry 3 git push https://${GITHUB_USERNAME_SECRET}:${GITHUB_TOKEN}@github.com/${owner}/${repository}.git "${branch}"
}

Maybe, it would not be needed to run those git config commands.

The same would happen for the elastic-package: elastic/elastic-package#1942

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried to remove this git config lines and test it, but it is not work and ask to set this parameters.
So I will leave it like it is for now.

@jsoriano
Copy link
Member

test integrations

@elastic-vault-github-plugin-prod

Created or updated PR in integrations repository to test this version. Check elastic/integrations#10559

@ev1yehor
Copy link
Contributor Author

test integrations

@ev1yehor
Copy link
Contributor Author

test integrations

@elastic-vault-github-plugin-prod

Created or updated PR in integrations repository to test this version. Check elastic/integrations#10559

@ev1yehor
Copy link
Contributor Author

Changes was tested. https://buildkite.com/elastic/package-spec-test-with-integrations/builds/43#0190df57-e9b7-4bc6-ae17-8a7cc3aac072
Once checks will be green, can be reviewed.

@elasticmachine
Copy link

💚 Build Succeeded

History

cc @ev1yehor

@ev1yehor ev1yehor marked this pull request as ready for review July 23, 2024 11:38
@ev1yehor ev1yehor requested review from jsoriano and mrodm July 23, 2024 12:42
Copy link
Member

@jsoriano jsoriano left a comment

Choose a reason for hiding this comment

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

Thanks @ev1yehor!

@jsoriano jsoriano merged commit 8e68ec8 into elastic:main Jul 23, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants