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

Prepare Cloudbeat for AWS Gov cloud #2050

Merged
merged 10 commits into from
Mar 26, 2024
Merged

Prepare Cloudbeat for AWS Gov cloud #2050

merged 10 commits into from
Mar 26, 2024

Conversation

kubasobon
Copy link
Member

@kubasobon kubasobon commented Mar 20, 2024

Summary of your changes

  • Added handling of AWS Gov cloud's default region us-gov-east-1
  • Improved existing fetcher clients to handle Gov regions and avoid nil pointer dereference errors

Screenshot/Data

Tested in AWS Gov cloud manually.
image

Related Issues

Fixes https://github.com/elastic/security-team/issues/8681

Checklist

  • I have added tests that prove my fix is effective or that my feature works
  • I have added the necessary README/documentation (if appropriate)

Introducing a new rule?

Copy link

mergify bot commented Mar 20, 2024

This pull request does not have a backport label. Could you fix it @kubasobon? 🙏
To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

  • backport-v./d./d./d is the label to automatically backport to the 8./d branch. /d is the digit
    NOTE: backport-skip has been added to this pull request.

Copy link

github-actions bot commented Mar 20, 2024

📊 Allure Report - 💚 No failures were reported.

Result Count
🟥 Failed 0
🟩 Passed 162
⬜ Skipped 0

@kubasobon kubasobon marked this pull request as ready for review March 25, 2024 15:48
@kubasobon kubasobon requested a review from a team as a code owner March 25, 2024 15:48
)

awsConfig, awsIdentity, err = a.getIdentity(ctx, cfg)
// TODO(kuba): Ask when the DefaultRegion is empty. Is there a chance it is
Copy link
Member

Choose a reason for hiding this comment

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

This needs to be solved before merging, right?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, exactly. I would love someone more knowledgable would comment on how CloudConfig.Aws is populated.

Copy link
Member

Choose a reason for hiding this comment

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

someone more knowledgable would comment on how CloudConfig.Aws is populated.

I don't qualify as very knowledgable, but afaik this configuration comes from the configuration yaml file + injected via agent.

I don't think overwriting is good idea.

But @amirbenun or @jeniawhite are better suited to explain how it's populated and if it's wise to overwrite it

Copy link
Contributor

Choose a reason for hiding this comment

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

@kubasobon @romulets
The configuration comes from the agent and is based on the populated config during the installation of the AWS CSPM integration.
so it appears that the DefaultRegion field is not being populated.

Copy link
Member Author

Choose a reason for hiding this comment

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

Perfect, so there is no chance of getting a collision. Thank you.

internal/flavors/benchmark/aws.go Show resolved Hide resolved
@kubasobon
Copy link
Member Author

Tested on regular AWS as well:
Screenshot 2024-03-26 at 13 08 59

@kubasobon kubasobon enabled auto-merge (squash) March 26, 2024 12:13
@kubasobon kubasobon merged commit 9e7920d into elastic:main Mar 26, 2024
27 checks passed
@kubasobon kubasobon deleted the aws-gov branch March 26, 2024 12:36
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