Move `fips_enabled` setting to AWS Common #28899

legoguy1000 · 2021-11-09T22:25:30Z

What does this PR do?

Moves the fips_enabled config option from the aws-s3 input to the AWS Common module to allow for any AWS related module to be able to use FIPS endpoints. Also updated all FIPS capable services to use the above config to set the endpoint. The only service that didn't support FIPS was tagging. Also adds TLS configuration to the AWS common module.

Why is it important?

Currently with the aws-s3 input the S3 bucket operations are able to use the FIPS endpoints but there is a single SQS API call that doesn't utilize FIPS if the setting is configured. Also the Cloudwatch input nor the AWS Metricbeat module are able to use FIPS currently. Reference https://discuss.elastic.co/t/sqs-client-ignores-the-fips-flag-for-the-service-endpoint/288687

Checklist

My code follows the style guidelines of this project
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
I have made corresponding change to the default configuration files
I have added tests that prove my fix is effective or that my feature works
I have added an entry in CHANGELOG.next.asciidoc or CHANGELOG-developer.next.asciidoc.

Author's Checklist

[ ]

How to test this PR locally

Related issues

Use cases

Screenshots

Logs

mergify · 2021-11-09T22:25:37Z

This pull request does not have a backport label. Could you fix it @legoguy1000? 🙏
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 7./d branch. /d is the digit

NOTE: backport-skip has been added to this pull request.

legoguy1000 · 2021-11-09T22:27:46Z

@kaiyan-sheng THoughts? Changes from the Discuss post above.

elasticmachine · 2021-11-09T22:30:46Z

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

Start Time: 2021-12-14T02:25:27.098+0000
Duration: 142 min 24 sec
Commit: 79e29a7

Test stats 🧪

Test	Results
Failed	0
Passed	28307
Skipped	3038
Total	31345

💚 Flaky test report

Tests succeeded.

🤖 GitHub comments

To re-run your PR in the CI, just comment with:

/test : Re-trigger the build.
/package : Generate the packages and run the E2E tests.
/beats-tester : Run the installation tests with beats-tester.
run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

kaiyan-sheng · 2021-11-09T22:46:07Z

@legoguy1000 +1 on moving this config to aws common! Thanks for working on it!

legoguy1000 · 2021-11-10T14:31:30Z

I think I just need to update the docs and default config files and then should be ready for review.

legoguy1000 · 2021-11-11T16:57:15Z

I did find 1 thing that may be problematic. If you look at https://aws.amazon.com/compliance/fips/, all the US East/West regions support the -fips endpoints but for the US GovCloud regions, some add the -fips and some don't. The SQS FIPS endpoint for GovCloud is sqs.us-gov-..... but the S3 is s3-fips.us-gov-.... so the current logic would break because sqs-fips.us-gov..... endpoint doesn't exist. I can add logic to check for that if we think that makes sense?

elasticmachine · 2021-11-12T15:41:37Z

Pinging @elastic/integrations (Team:Platforms)

legoguy1000 · 2021-11-13T18:10:09Z

@kaiyan-sheng I think its ready for review. I added a new TLS config to the AWS common module so if there is a need to change the trusted root certs, set insecure, ..... (mainly for self hosted/3rd party services). Also can be used for testing with something like https://localstack.cloud/.

mergify · 2021-11-15T21:36:15Z

This pull request is now in conflicts. Could you fix it? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

git fetch upstream
git checkout -b aws-fips upstream/aws-fips
git merge upstream/master
git push upstream aws-fips

mergify · 2021-11-18T08:44:53Z

This pull request is now in conflicts. Could you fix it? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

git fetch upstream
git checkout -b aws-fips upstream/aws-fips
git merge upstream/master
git push upstream aws-fips

kaiyan-sheng · 2021-11-22T14:51:23Z

@legoguy1000 Sorry I just came back from vacation. Will review this today!!

kaiyan-sheng · 2021-11-22T14:51:34Z

/test

kaiyan-sheng · 2021-11-22T22:53:14Z

Overall it looks good to me besides make update needs to be ran everywhere seems like 😬 @leehinman Could you also take a look at this PR please?

legoguy1000 · 2021-11-23T03:15:24Z

@kaiyan-sheng I think i ran all the make updates that I needed too.

leehinman

Nice.

One minor comment below. I'm ok with merging if the comment doesn't resonate with you.

leehinman · 2021-11-23T17:49:23Z

x-pack/libbeat/common/aws/credentials.go

+func CreateServiceName(serviceName string, fipsEnabled bool, region string) string {
+	if fipsEnabled {
+		OptionalGovCloudFIPS := []string{"s3"}
+		_, found := Find(OptionalGovCloudFIPS, serviceName)


Minor nit. If you switch to a Map, you can get rid of the Find function. Also might be nice to have the OptionalGovCloudFIPS var be package level with a godoc comment.

I can change it to a map.

as for where to put the variable, i'm not a GoLang expert to know what is best but I can't see the variable being used outside of this function.

I added the map

Reasoning behind moving variable to top level is for doc comments That way the comment shows up in the godoc output. Again not a big deal, and I'm ok with merging without moving.

I think i moved it correctly. Please let me know otherwise.

mergify · 2021-12-01T17:58:55Z

This pull request is now in conflicts. Could you fix it? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

git fetch upstream
git checkout -b aws-fips upstream/aws-fips
git merge upstream/master
git push upstream aws-fips

mergify · 2021-12-06T19:46:01Z

This pull request is now in conflicts. Could you fix it? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

git fetch upstream
git checkout -b aws-fips upstream/aws-fips
git merge upstream/master
git push upstream aws-fips

kaiyan-sheng · 2021-12-06T21:14:47Z

/test

legoguy1000 · 2021-12-06T23:42:03Z

@kaiyan-sheng @leehinman Anything more I need to do for this PR?

kaiyan-sheng · 2021-12-07T15:35:02Z

/test

kaiyan-sheng · 2021-12-07T16:12:27Z

@legoguy1000 Sorry I fixed the merge conflict and then realized lint is not happy 😂 Could you take a look when you get a chance please? TIA!!

legoguy1000 · 2021-12-07T18:35:53Z

@legoguy1000 Sorry I fixed the merge conflict and then realized lint is not happy 😂 Could you take a look when you get a chance please? TIA!!

Are you able to tell what's causing the linter to fail?

legoguy1000 · 2021-12-07T18:45:01Z

@legoguy1000 Sorry I fixed the merge conflict and then realized lint is not happy 😂 Could you take a look when you get a chance please? TIA!!

Are you able to tell what's causing the linter to fail?

nevermind, I think I found it.

legoguy1000 · 2021-12-07T22:14:09Z

@kaiyan-sheng Looks like AWS tests had issues due to the us-east-1 region issues today. May need to rerun the tests.

kaiyan-sheng · 2021-12-08T03:35:16Z

/test

mergify · 2021-12-09T15:33:39Z

This pull request is now in conflicts. Could you fix it? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

git fetch upstream
git checkout -b aws-fips upstream/aws-fips
git merge upstream/master
git push upstream aws-fips

mergify · 2021-12-13T20:58:58Z

This pull request is now in conflicts. Could you fix it? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

git fetch upstream
git checkout -b aws-fips upstream/aws-fips
git merge upstream/master
git push upstream aws-fips

(cherry picked from commit 377f97b)

) (cherry picked from commit 377f97b) Co-authored-by: Alex Resnick <[email protected]> Co-authored-by: kaiyan-sheng <[email protected]>

botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Nov 9, 2021

mergify bot added the backport-skip Skip notification from the automated backport with mergify label Nov 9, 2021

legoguy1000 force-pushed the aws-fips branch from cb6bf61 to 29133d0 Compare November 9, 2021 22:25

mtojek added the Team:Platforms Label for the Integrations - Platforms team label Nov 10, 2021

botelastic bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Nov 10, 2021

legoguy1000 marked this pull request as ready for review November 12, 2021 15:41

legoguy1000 force-pushed the aws-fips branch from 2d8c0f9 to a305266 Compare November 13, 2021 18:08

legoguy1000 force-pushed the aws-fips branch from a305266 to bf1d306 Compare November 13, 2021 18:14

legoguy1000 force-pushed the aws-fips branch from b465c0b to eb43944 Compare November 16, 2021 20:46

legoguy1000 force-pushed the aws-fips branch from eb43944 to ef94728 Compare November 18, 2021 23:13

kaiyan-sheng requested a review from leehinman November 22, 2021 22:35

leehinman reviewed Nov 23, 2021

View reviewed changes

legoguy1000 force-pushed the aws-fips branch from 4769881 to 7f82a22 Compare December 2, 2021 01:25

kaiyan-sheng added the backport-v8.1.0 Automated backport with mergify label Dec 6, 2021

mergify bot removed the backport-skip Skip notification from the automated backport with mergify label Dec 6, 2021

kaiyan-sheng approved these changes Dec 7, 2021

View reviewed changes

kaiyan-sheng added backport-v8.0.0 Automated backport with mergify and removed backport-v8.1.0 Automated backport with mergify labels Dec 7, 2021

legoguy1000 added 5 commits December 14, 2021 02:24

Move fips_enabled setting to AWS Common

9ea54ef

fix issues?

80b24ae

replace list with map

e94173a

move variable to top level

92f1ef3

fix linting

79e29a7

legoguy1000 force-pushed the aws-fips branch from 03146e3 to 79e29a7 Compare December 14, 2021 02:25

kaiyan-sheng self-assigned this Dec 14, 2021

kaiyan-sheng merged commit 377f97b into elastic:master Dec 14, 2021

mergify bot pushed a commit that referenced this pull request Dec 14, 2021

Move fips_enabled setting to AWS Common (#28899)

5aa00eb

(cherry picked from commit 377f97b)

mergify bot mentioned this pull request Dec 14, 2021

[8.0](backport #28899) Move fips_enabled setting to AWS Common #29418

Merged

legoguy1000 deleted the aws-fips branch December 14, 2021 12:22

kaiyan-sheng added a commit that referenced this pull request Dec 15, 2021

[8.0](backport #28899) Move fips_enabled setting to AWS Common (#29418

518b482

) (cherry picked from commit 377f97b) Co-authored-by: Alex Resnick <[email protected]> Co-authored-by: kaiyan-sheng <[email protected]>

Move fips_enabled setting to AWS Common #28899

Move fips_enabled setting to AWS Common #28899

Conversation

legoguy1000 commented Nov 9, 2021 • edited Loading

What does this PR do?

Why is it important?

Checklist

Author's Checklist

How to test this PR locally

Related issues

Use cases

Screenshots

Logs

mergify bot commented Nov 9, 2021

legoguy1000 commented Nov 9, 2021

elasticmachine commented Nov 9, 2021 • edited by jenkins-beats-ci bot Loading

💚 Build Succeeded

Build stats

Test stats 🧪

💚 Flaky test report

🤖 GitHub comments

kaiyan-sheng commented Nov 9, 2021

legoguy1000 commented Nov 10, 2021

legoguy1000 commented Nov 11, 2021

elasticmachine commented Nov 12, 2021

legoguy1000 commented Nov 13, 2021

mergify bot commented Nov 15, 2021

mergify bot commented Nov 18, 2021

kaiyan-sheng commented Nov 22, 2021

kaiyan-sheng commented Nov 22, 2021

kaiyan-sheng commented Nov 22, 2021

legoguy1000 commented Nov 23, 2021

leehinman left a comment

Choose a reason for hiding this comment

leehinman Nov 23, 2021

Choose a reason for hiding this comment

legoguy1000 Nov 23, 2021

Choose a reason for hiding this comment

legoguy1000 Nov 24, 2021

Choose a reason for hiding this comment

legoguy1000 Nov 29, 2021

Choose a reason for hiding this comment

leehinman Dec 1, 2021

Choose a reason for hiding this comment

legoguy1000 Dec 2, 2021

Choose a reason for hiding this comment

mergify bot commented Dec 1, 2021

mergify bot commented Dec 6, 2021

kaiyan-sheng commented Dec 6, 2021

legoguy1000 commented Dec 6, 2021

kaiyan-sheng commented Dec 7, 2021

kaiyan-sheng commented Dec 7, 2021

legoguy1000 commented Dec 7, 2021

legoguy1000 commented Dec 7, 2021

legoguy1000 commented Dec 7, 2021

kaiyan-sheng commented Dec 8, 2021

mergify bot commented Dec 9, 2021

mergify bot commented Dec 13, 2021

Move `fips_enabled` setting to AWS Common #28899

Move `fips_enabled` setting to AWS Common #28899

legoguy1000 commented Nov 9, 2021 •

edited

Loading

elasticmachine commented Nov 9, 2021 •

edited by jenkins-beats-ci bot

Loading