Skip to content

Generate STS endpoints to replace hard-coded lists#31217

Closed
greedy52 wants to merge 1 commit intomasterfrom
STeve/generate_sts_endpoints
Closed

Generate STS endpoints to replace hard-coded lists#31217
greedy52 wants to merge 1 commit intomasterfrom
STeve/generate_sts_endpoints

Conversation

@greedy52
Copy link
Copy Markdown
Contributor

This is an alternative to

Logic is copied from https://github.com/nklaassen/sts-endpoints with some minor modifications.

Note that go.mod must be updated with a newer version of aws-sdk-go to get newer regions on older release branches.

@greedy52 greedy52 self-assigned this Aug 30, 2023
@github-actions github-actions Bot requested review from atburke and avatus August 30, 2023 16:47
@tigrato
Copy link
Copy Markdown
Contributor

tigrato commented Aug 30, 2023

Since we do not upgrade deps 😢 on old branches, can we do something like:

  • We fork @nklaassen tool into our codebase
  • We add a ci step that:
    • Creates the go.mod with the most recent version of AWS SDK
    • Generating the list confirms that there are no differences between the existing and the new version.
    • If diff isn't empty, we abort and report a failure saying - Look fix this with make fix-sts-endpoints

With this, we can keep older branches up-to-date without having to upgrade aws SDK as long as older AWS SDKs support new regions/sts endpoints. We can run the step using a crontab to ensure it's updated

What do you think @greedy52 ?

@reedloden
Copy link
Copy Markdown
Contributor

We update dependencies for security updates. Could we not also update certain other dependencies on an allowlist / case-by-case basis? Seems like a lot of extra complexity that could be avoided, while still working to keep certain stability "promises".

@tigrato
Copy link
Copy Markdown
Contributor

tigrato commented Aug 30, 2023

We update dependencies for security updates. Could we not also update certain other dependencies on an allowlist / case-by-case basis? Seems like a lot of extra complexity that could be avoided, while still working to keep certain stability "promises".

I agree with this but I had the impression that the security updates were manual and nothing automatic runs.

we can set that for release branches - it might involve some work when dealing with updates that break features or with major version updates but we can probably restrict the updates to minor versions only.

@greedy52
Copy link
Copy Markdown
Contributor Author

Since we do not upgrade deps 😢 on old branches, can we do something like:

  • We fork @nklaassen tool into our codebase

  • We add a ci step that:

    • Creates the go.mod with the most recent version of AWS SDK
    • Generating the list confirms that there are no differences between the existing and the new version.
    • If diff isn't empty, we abort and report a failure saying - Look fix this with make fix-sts-endpoints

With this, we can keep older branches up-to-date without having to upgrade aws SDK as long as older AWS SDKs support new regions/sts endpoints. We can run the step using a crontab to ensure it's updated

What do you think @greedy52 ?

Here is another case relying on newer SDKs for regions:

// awsRegions returns the list of all regions available on every aws partition.
// It is used to validate the AWSMatcher.Regions values.
func awsRegions() []string {
var regions []string
partitions := endpoints.DefaultPartitions()
for _, partition := range partitions {
regions = append(regions, maps.Keys(partition.Regions())...)
}
return regions
}

Whatever we choose, we have to do for both.

@nklaassen
Copy link
Copy Markdown
Contributor

For history, I originally wrote this feature to dynamically generate these from the SDK within Teleport, switched to a static list based on this review discussion #15337 (comment)

I do have dependabot+CI set up in https://github.com/nklaassen/sts-endpoints that alerted me that there is a new region, I would have opened a PR if I wasn't on paternity leave and had seen the email. But I don't object to pulling this directly into a Teleport CI step.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants