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

Uncluster SAR release from tag release, and place it along ESF terraform dependencies #809

Merged
merged 5 commits into from
Sep 25, 2024

Conversation

constanca-m
Copy link
Contributor

What and why

We have two jobs that are triggered by a new tag release and involve a new ESF production release:

  • SAR release
  • Uploading dependencies for ESF terraform

These two jobs are placed in different workflows. This PR moves both of them to the same one. I am also renaming the release workflow to create-tag, since I believe the naming cause this confusion and it is more clear this way.

Check issue #807.

Signed-off-by: constanca <[email protected]>
@constanca-m constanca-m requested review from zmoog and v1v September 24, 2024 10:08
@constanca-m constanca-m self-assigned this Sep 24, 2024
Signed-off-by: constanca <[email protected]>
Signed-off-by: constanca <[email protected]>
---
# Workflow to push zip with dependencies to S3 bucket every time the ESF version is updated
# (we need this for ESF terraform), and to publish the new SAR version
name: releases-production
Copy link
Member

Choose a reason for hiding this comment

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

This will require a change in the infra part, so the terraform definition for the OIDC is configured for the releases-production

Suggested change
name: releases-production
# If you change the name, please adjust the changes in https://github.com/elastic/oblt-infra/tree/main/conf/resources/repos/elastic-serverless-forwarder
name: releases-production

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This will require a change in the infra part, so the terraform definition for the OIDC is configured for the create-tag

Wouldn't it be better to create a role in the AWS account? This way the team could change the permissions if necessary. So:

  1. A role for the dependencies bucket (that part is done and was working)
  2. A role for the SAR

Copy link
Member

Choose a reason for hiding this comment

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

Wouldn't it be better to create a role in the AWS account? This way the team could change the permissions if necessary. So:

I understand your question about flexibility, but we are not using that approach anymore for various security reasons:

  • Changes are not tracked through IasC but click-ops
  • No PRs or code changes in place that can be reviewed
  • Eventually remove the permissions to AWS prod and use IasC for it

In other words, there should be a separation of concerns when running things in the CI. I'll ping you to the original request to do the separation of concerns when accessing cloud accounts in the CI.

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 understand, and it makes sense. In that case, for this PR to work:

  1. We need to update this with the correct filename like you mentioned.
  2. We need to create a PR to add a new file for the role for the ESF dependencies. How to do this?
  3. Remove the role we are using to upload the dependencies in this job, since 2. already takes care of that.

Copy link
Contributor Author

@constanca-m constanca-m Sep 24, 2024

Choose a reason for hiding this comment

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

I found this guide, but I don't understand how the job will ever know about this *.tf file. And would the role assumed scope to the job? Can we have 2 different roles, one being used for each job, for the same workflow?

Copy link
Member

@v1v v1v Sep 24, 2024

Choose a reason for hiding this comment

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

I don't understand how the job will ever know about this *.tf file.

There are two parts:

  1. The GitHub workflow using the id-token: write and https://github.com/elastic/oblt-actions/tree/main/aws/auth
  2. The AWS OIDC configuration using the *.tf files

2) needs to be applied before 1) can run, for such, it's required to list all the required permissions so it can be configured accordingly. Those *.tf files are the ones saying what's the permissions for the given GitHub workflow. To apply tho *.tf files, that's only possible by a few people for the reasons I mentioned above.

And would the role assumed scope to the job? Can we have 2 different roles, one being used for each job, for the same workflow?

Let me ping @reakaleek; he did all the heavy lifting and can answer your questions better than I can.

Copy link
Member

@reakaleek reakaleek Sep 24, 2024

Choose a reason for hiding this comment

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

Hi @constanca-m,

You can find our official documentation about our keyless setup at https://github.com/elastic/observability-robots/blob/main/docs/teams/ci/keyless/github-actions/amazon-web-services.md

And would the role assumed scope to the job? Can we have 2 different roles, one being used for each job, for the same workflow?

Unfortunately, we don't differentiate between jobs in a workflow. We only separate between workflows. Essentially, we set up OIDC for a workflow. For each workflow that is defined, a role is created which can be assumed by the workflow.

In the linked documentation, you can find how we are attaching policies to a given role.

Hence, in this case we would attach the permissions needed for both jobs to a single role.

If you think it's crucial to give each job in a workflow different permissions, then let's discuss, and we can try to figure out how to extend the existing functionality to also accomodate this need.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for the detailed answer. Since this is just the permissions to execute the workflow, I don't think there is an issue with granting both jobs permissions to just one role.

I have opened https://github.com/elastic/oblt-infra/pull/194 to fix this issue.

Signed-off-by: constanca <[email protected]>
Signed-off-by: constanca <[email protected]>
@constanca-m constanca-m merged commit ae60256 into elastic:main Sep 25, 2024
4 checks passed
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.

4 participants