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

Don't force the use of FIPS endpoints for DynamoDB Streams and Application Auto Scaling #34876

Merged
merged 1 commit into from
Nov 29, 2023

Conversation

reedloden
Copy link
Contributor

@reedloden reedloden commented Nov 22, 2023

DynamoDB Streams and Application Auto Scaling do not currently have FIPS endpoints in non-GovCloud, leading to invalid endpoints for FIPS users running in AWS Standard.

See also: https://aws.amazon.com/compliance/fips/#FIPS_Endpoints_by_Service

Regression from #34170.

Fixes #34804.

Additionally, clean-up a few more AWS session initiations to be consistent and clear.

changelog: Don't force the use of FIPS endpoints for DynamoDB Streams and Application Auto Scaling

Copy link

The PR changelog entry failed validation: Changelog entry not found in the PR body. Please add a "no-changelog" label to the PR, or changelog lines starting with changelog: followed by the changelog entries for the PR.

@reedloden reedloden force-pushed the reed/dynamodbstreams-fips-endpoint branch 2 times, most recently from 65fd404 to 8aee840 Compare November 22, 2023 08:49
Copy link
Contributor

@rosstimothy rosstimothy left a comment

Choose a reason for hiding this comment

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

Could you please add some tests that validate the behavior change?

@GavinFrazar
Copy link
Contributor

LGTM but I didn't test it.

I think the same issue can happen with AWS Application Auto Scaling though, since it also does not have fips endpoints in non-gov cloud.

We use that here:

// Enable auto scaling if requested.
if b.Config.EnableAutoScaling {
if err := SetAutoScaling(ctx, applicationautoscaling.New(b.session), GetTableID(b.TableName), AutoScalingParams{
ReadMinCapacity: b.Config.ReadMinCapacity,
ReadMaxCapacity: b.Config.ReadMaxCapacity,
ReadTargetValue: b.Config.ReadTargetValue,
WriteMinCapacity: b.Config.WriteMinCapacity,
WriteMaxCapacity: b.Config.WriteMaxCapacity,
WriteTargetValue: b.Config.WriteTargetValue,
}); err != nil {
return nil, trace.Wrap(err)
}
}

If you've already tested this could you try setting auto_scaling: true in your teleport.storage dynamodb config?

@reedloden reedloden force-pushed the reed/dynamodbstreams-fips-endpoint branch from 8aee840 to 4a50d18 Compare November 25, 2023 07:24
@reedloden
Copy link
Contributor Author

Could you please add some tests that validate the behavior change?

I flipped the logic so that we only force FIPS endpoints for the services we know support it (mainly DynamoDB in this case). This obliviated the need for extra complexity, so I don't think a separate test is needed.

@reedloden
Copy link
Contributor Author

I think the same issue can happen with AWS Application Auto Scaling though, since it also does not have fips endpoints in non-gov cloud.

Good catch. As mentioned above, I flipped the logic so that we're only forcing FIPS endpoints for DynamoDB in this case (and not DynamoDB Streams or Application Auto Scaling). So, that should address this issue.

@reedloden reedloden force-pushed the reed/dynamodbstreams-fips-endpoint branch 5 times, most recently from 94050d1 to e453ffa Compare November 25, 2023 07:50
@reedloden reedloden force-pushed the reed/dynamodbstreams-fips-endpoint branch 2 times, most recently from 738777a to 642665f Compare November 25, 2023 08:38
@reedloden reedloden changed the title Don't force the use of FIPS endpoints for DynamoDB Streams in AWS Standard regions Don't force the use of FIPS endpoints for DynamoDB Streams and Application Auto Scaling Nov 25, 2023
Copy link
Contributor

@rosstimothy rosstimothy left a comment

Choose a reason for hiding this comment

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

Tests would still be useful to verify that we are using the correct endpoints to prevent regressions.

lib/backend/dynamo/dynamodbbk.go Outdated Show resolved Hide resolved
@reedloden reedloden force-pushed the reed/dynamodbstreams-fips-endpoint branch from 642665f to 970bca8 Compare November 27, 2023 22:14
…ation Auto Scaling

DynamoDB Streams and Application Auto Scaling do not currently have FIPS endpoints in
non-GovCloud, leading to invalid endpoints for FIPS users running in AWS Standard.

See also: https://aws.amazon.com/compliance/fips/#FIPS_Endpoints_by_Service

Regression from #34170.

Fixes #34804.

Additionally, clean-up a few more AWS session initiations to be consistent and clear.
@reedloden reedloden force-pushed the reed/dynamodbstreams-fips-endpoint branch from 970bca8 to 4a1017b Compare November 29, 2023 21:00
@reedloden reedloden added this pull request to the merge queue Nov 29, 2023
Merged via the queue into master with commit 6cd68f0 Nov 29, 2023
35 checks passed
@reedloden reedloden deleted the reed/dynamodbstreams-fips-endpoint branch November 29, 2023 21:44
@public-teleport-github-review-bot

@reedloden See the table below for backport results.

Branch Result
branch/v14 Create PR

reedloden added a commit that referenced this pull request Nov 29, 2023
…Application Auto Scaling

Backport of #34876.

DynamoDB Streams and Application Auto Scaling do not currently have FIPS endpoints in
non-GovCloud, leading to invalid endpoints for FIPS users running in AWS Standard.

See also: https://aws.amazon.com/compliance/fips/#FIPS_Endpoints_by_Service

Regression from #34170.

Fixes #34804.

Additionally, clean-up a few more AWS session initiations to be consistent and clear.
reedloden added a commit that referenced this pull request Nov 29, 2023
…Application Auto Scaling

Backport of #34876.

DynamoDB Streams and Application Auto Scaling do not currently have FIPS endpoints in
non-GovCloud, leading to invalid endpoints for FIPS users running in AWS Standard.

See also: https://aws.amazon.com/compliance/fips/#FIPS_Endpoints_by_Service

Regression from #34170.

Fixes #34804.

Additionally, clean-up a few more AWS session initiations to be consistent and clear.
reedloden added a commit that referenced this pull request Nov 29, 2023
…Application Auto Scaling

Backport of #34876.

DynamoDB Streams and Application Auto Scaling do not currently have FIPS endpoints in
non-GovCloud, leading to invalid endpoints for FIPS users running in AWS Standard.

See also: https://aws.amazon.com/compliance/fips/#FIPS_Endpoints_by_Service

Regression from #34170.

Fixes #34804.

Additionally, clean-up a few more AWS session initiations to be consistent and clear.
reedloden added a commit that referenced this pull request Nov 29, 2023
…Application Auto Scaling

Backport of #34876.

DynamoDB Streams and Application Auto Scaling do not currently have FIPS endpoints in
non-GovCloud, leading to invalid endpoints for FIPS users running in AWS Standard.

See also: https://aws.amazon.com/compliance/fips/#FIPS_Endpoints_by_Service

Regression from #34170.

Fixes #34804.

Additionally, clean-up a few more AWS session initiations to be consistent and clear.
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.

Not able to use FIPS in AWS non-GovCloud
4 participants