Skip to content
This repository has been archived by the owner on Mar 2, 2023. It is now read-only.

Added dynamic bucket region session option #4

Merged
merged 5 commits into from
May 20, 2021

Conversation

pregnor
Copy link
Member

@pregnor pregnor commented May 20, 2021

Q A
Bug fix? no
New feature? yes
API breaks? no
Deprecations? no
Related tickets -
License Apache 2.0

What's in this PR?

  1. Added an AWS session initialization option for dynamically retrieving the region of the S3 bucket which is used in the helm command.
  2. Added unit test cases covering all paths of this option.
  3. Updated the calls to the session constructor to use this option.
  4. Added end to end test cases for fetch operation using dynamic bucket region.
  5. Fixed linter issues in the modified packages to ensure passing checks.

The HEAD {{bucket}} request allows querying the bucket's actual region without prior knowledge with the proper configuration.

Unfortunately this behavior is not explicitly documented - or at least I couldn't find it - but the AWS SDK Go team confirmed in an issue comment this is an intended and supported behavior of the endpoint.

If the option runs into any issue, it has no effect on the session/configuration as a fallback mechanism. In such a case the plugin's behavior remains the same as before, namely falling back to the HELM_S3_REGION environment variable's value or to the corresponding AWS environment variable's value (AWS_REGION or AWS_DEFAULT_REGION).

Why?

Without the correct region being set currently the plugin throws an error:

fetch from s3 uri=s3://pregnor-helm-s3-repo-test/stable/cadence/cadence-0.18.0.tgz: fetch object from s3: BucketRegionError: incorrect region, the bucket is not in 'us-west-2' region at endpoint ''
        status code: 301, request id: , host id: 
Error: plugin "bin/helms3" exited with error

We wanted to allow the plugin to work without manual region setting and also support multiple repositories residing in different regions without updating the regions in-between fetching charts from those repositories. This is especially relevant when the plugin is used in a Terraform environment.

Additional context

The buckets used in the unit tests were found randomly, they are not mine and I do not have access to them, but they fit the test cases perfectly.

Checklist

  • Code meets the Developer Guide
  • User guide and development docs updated (if needed)
  • Related Helm chart(s) updated (if needed)

@pregnor pregnor added the enhancement New feature or request label May 20, 2021
@pregnor pregnor self-assigned this May 20, 2021
@pregnor pregnor force-pushed the feat/dynamic-bucket-region-2 branch 3 times, most recently from b951050 to ca7ea2f Compare May 20, 2021 09:35
Implemented a session option which dynamically
determines the AWS S3 bucket's region without
prior static manual setting based on the HEAD
bucket response.
Added a unit test for the dynamic bucket region
option.
@pregnor pregnor force-pushed the feat/dynamic-bucket-region-2 branch from b8c0074 to 58e83c2 Compare May 20, 2021 11:52
@pregnor pregnor merged commit b617238 into main May 20, 2021
@pregnor pregnor deleted the feat/dynamic-bucket-region-2 branch May 20, 2021 12:02
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant