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

Updating cicd #169

Merged
merged 16 commits into from
Aug 1, 2023
Merged

Updating cicd #169

merged 16 commits into from
Aug 1, 2023

Conversation

LikeTheSalad
Copy link
Contributor

No description provided.

Co-authored-by: Ivan Fernandez Calvo <kuisathaverat@users.noreply.github.com>
@kuisathaverat
Copy link
Contributor

@elastic/ci-robots Could you review this one?

@LikeTheSalad
Copy link
Contributor Author

There are a lot of files in this PR that are specific to this repo's custom functionality, except for the .github/workflows/release.yml one, which is the one I wasn't too sure about since I'm not aware of the details of the apmmachine Git auth functionality. So since it seems like, at least from a general perspective, there are no apparent issues that you see with it @kuisathaverat, I'm inclined to move forward with merging these changes, then try them out in a new release and, if the Git auth fails, I think we can then create a PR that would be focused only on the release.yml file, and that way it might be more reviewer-friendly for anyone not aware of the specifics of this repo.

@kuisathaverat kuisathaverat requested a review from a team June 19, 2023 07:55
Comment on lines 5 to 14
- uses: elastic/apm-pipeline-library/.github/actions/github-token@current
with:
url: ${{ secrets.VAULT_ADDR }}
roleId: ${{ secrets.VAULT_ROLE_ID }}
secretId: ${{ secrets.VAULT_SECRET_ID }}
- uses: elastic/apm-pipeline-library/.github/actions/setup-git@current
with:
username: ${{ env.GIT_USER }}
email: ${{ env.GIT_EMAIL }}
token: ${{ env.GITHUB_TOKEN }}
Copy link
Member

Choose a reason for hiding this comment

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

Sorry if I created any confusion.
However, this is GH workflow syntax. This won't work in a buildkite pipeline.

Copy link
Member

Choose a reason for hiding this comment

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

Here is example on how secrets are currently acquired from vault.

PLUGIN_PORTAL_KEY=$(vault read secret/release/gradle-plugin-portal -format=json | jq -r .data.key)

You would need to get the token in the same manner.

Unfortunately, I don't know if there is a token for apmmachine stored in the ci vault.

@elastic/observablt-ci anyone of you knows how to get the secret for apmmachine in buildkite?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh! I see, tbh I thought it seemed too easy to be true 😅 - I've been mostly looking at how this has been done in the Java Agent repo. And after a closer look, it seems like the code change pushed into main is done outside of Buildkite, so I'm thinking that maybe that can be done here as well, in case getting this token inside Buildkite might not be worth the trouble... So I'll take a look at that alternative in the meantime, cheers.

Copy link
Contributor

Choose a reason for hiding this comment

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

@elastic/observablt-ci anyone of you knows how to get the secret for apmmachine in buildkite?

Buildkite can retrieve secrets from another vault.
I will send the documentation by DM since it's internal.

Copy link
Contributor Author

@LikeTheSalad LikeTheSalad Jul 14, 2023

Choose a reason for hiding this comment

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

I just moved the post-deploy command to the GH release.yml file here alongside the GIT auth set up as well, I think it should work. Any thoughts @reakaleek @amannocci ?

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've added a new GH action job that should run after the release has been successful. In this new job we should do the version bump and changelog update and it should all get pushed straight into the main branch. I think it should work the way I wrote it, if there are any issues please let me know @amannocci @reakaleek - Otherwise I'm planning to merge this by the end of this week and apply future adjustments if needed since this PR has been open for too long now.

Copy link
Member

Choose a reason for hiding this comment

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

I think you need to set waitFor to true

so this works as expected

Copy link
Member

Choose a reason for hiding this comment

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

Another thing I noticed:

pushing to protected branches does not work out of the box in github actions.

You need to use a GH Personal Access Token that has elevated permissions.

Copy link
Member

@reakaleek reakaleek Jul 31, 2023

Choose a reason for hiding this comment

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

You can find an example job here with the necessary setup to do pushing to a protected branch:

https://github.com/elastic/apm-agent-java/blob/main/.github/workflows/release.yml#L128-L151

Also, we need to add @apmmachine as a collaborator with a role that is able to bypass branch protection.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Many thanks @reakaleek - I've just updated the job based on your suggestions. I hope it's better now. I'm not sure if @apmmachine already has that ability in this repo, although if that's not the case I'd request for it later.

# Conflicts:
#	agp-compatibility/agp-compatibility-7-2/metadata/notice.properties
#	agp-compatibility/agp-compatibility-7-3/metadata/notice.properties
#	agp-compatibility/agp-compatibility-api/metadata/notice.properties
#	android-common/metadata/notice.properties
#	android-instrumentation/metadata/notice.properties
#	android-plugin/metadata/notice.properties
#	android-sdk-ktx/metadata/notice.properties
#	android-sdk/metadata/notice.properties
…ne to push changes straight into the main branch
token: ${{ env.GITHUB_TOKEN }}
- uses: actions/checkout@v3
with:
ref: ${{ env.TAG_NAME }}
Copy link
Member

Choose a reason for hiding this comment

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

I guess this is a copy paste error. I think this should be something different.

maybe inputs.branch_specifier or just hardcoded main?

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 didn't notice, thanks for the heads-up! It should be fine now.

@LikeTheSalad LikeTheSalad merged commit 55c4406 into main Aug 1, 2023
2 checks passed
@LikeTheSalad LikeTheSalad deleted the updating-cicd branch August 1, 2023 07:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants