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

Missing region in AWS setup (e.g. datalink) #9284

Closed
2 tasks done
radeusgd opened this issue Mar 5, 2024 · 3 comments · Fixed by #9546
Closed
2 tasks done

Missing region in AWS setup (e.g. datalink) #9284

radeusgd opened this issue Mar 5, 2024 · 3 comments · Fixed by #9546
Assignees
Labels
--bug Type: bug -libs Libraries: New libraries to be implemented

Comments

@radeusgd
Copy link
Member

radeusgd commented Mar 5, 2024

I have noticed that because I've created a datalink to an S3 file, containing my AWS key credentials. So I should be able to read this datalink without setting up any additional credentials on my machine, right? Because the credentials are inside of the datalink itself.

Well, unfortunately - this uncovered that something is missing.
Reading the datalink I get:

(Error: (AWS_SDK_Error.Error 'Unable to load region from any of the providers in the chain software.amazon.awssdk.regions.providers.DefaultAwsRegionProviderChain@7a79a5d6: [software.amazon.awssdk.regions.providers.SystemSettingsRegionProvider@22decfa2: Unable to load region from system settings. Region must be specified either via environment variable (AWS_REGION) or  system property (aws.region)., software.amazon.awssdk.regions.providers.AwsProfileRegionProvider@4d50f682: No region provided in profile: default, software.amazon.awssdk.regions.providers.InstanceProfileRegionProvider@1315d910: Unable to contact EC2 metadata service.]'))
  • We need an ability to set region for all of our AWS operations (not only the datalinks).
    • I think it will be easiest to keep it part of the AWS_Credential although we could make it a separate property as well.
  • We should change our test setup.
    • By setting the default AWS environment variables on the CI, our tests run in a special environment. Our users may not have these environment variables set up, or they can point to different data. Tests need to be independent of that and ensure that e.g. the S3 data link has all the information needed to access the file and does not depend on user setup.
    • Let's rename the env vars to ENSO_AWS_* prefix, so that AWS tooling does not pick them up automatically. Then we can manually set up our AWS_Credential for use in the tests.
    • This will prove some difficulty with testing the AWS_Credential.Default variant.
@github-project-automation github-project-automation bot moved this to ❓New in Issues Board Mar 5, 2024
@radeusgd radeusgd added --bug Type: bug -libs Libraries: New libraries to be implemented labels Mar 5, 2024
@radeusgd radeusgd self-assigned this Mar 5, 2024
@radeusgd
Copy link
Member Author

radeusgd commented Mar 5, 2024

After discussion with @jdunkerley, we don't need to add the region to credentials yet.

For S3 which is global it is enough to default to any region (e.g. eu-west-1) if no region was specified. So we should add such a default (but ensure that if AWS_REGION or profile is set, we don't override that).

We still should adapt our tests as written above to catch such things in the future.

@AdRiley AdRiley moved this from ❓New to 📤 Backlog in Issues Board Mar 6, 2024
@radeusgd radeusgd moved this from 📤 Backlog to 🔧 Implementation in Issues Board Mar 25, 2024
@enso-bot
Copy link

enso-bot bot commented Mar 26, 2024

Radosław Waśko reports a new STANDUP for yesterday (2024-03-25):

Progress: Added default region to AWS Credentials. Turns out S3 API rejects connection if wrong region is specified. Work on resolving the region of a bucket - apparently this is much harder than it should be. Finally found a solution that seems to work. It should be finished by 2024-03-26.

Next Day: Next day I will be working on the same task. Adapt some tests of default credentials through allowing an override. Fix remaining issues and prepare a PR.

@radeusgd radeusgd moved this from 🔧 Implementation to 👁️ Code review in Issues Board Mar 26, 2024
@enso-bot
Copy link

enso-bot bot commented Mar 27, 2024

Radosław Waśko reports a new STANDUP for yesterday (2024-03-26):

Progress: Getting the pending PRs merged (one was failing due to case insensitivity of paths on Windows ignoring my rename...). Added some more tests for AWS_Credential in the S3 region PR, fixed an exception where AWS_Credential.profile_names was crashing if no AWS config was found. Implemented one more small fix to use new endpoints: #9551. It should be finished by 2024-03-26.

Next Day: Next day I will be working on the #8590 task. Move forward with work on types refactor. Start path-based Enso_File refactor.

@mergify mergify bot closed this as completed in #9546 Mar 27, 2024
mergify bot pushed a commit that referenced this issue Mar 27, 2024
- Closes #9284
- Now our tests run without the default `AWS_` config, thus ensuring that the tested setups work in a clean environment.
- After all, more complicated logic was needed for buckets access - apparently the AWS SDK only allows for some operations on buckets to happen if the client is connected to the correct region. Thus detection of bucket regions had to be implemented.
- Added `AWS_Region` widget based on autoscoping.
- Fixed `AWS_Credential.profile_names` crashing if no AWS config was found. Now it returns no profiles if not found. Added a regression test.
@github-project-automation github-project-automation bot moved this from 👁️ Code review to 🟢 Accepted in Issues Board Mar 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
--bug Type: bug -libs Libraries: New libraries to be implemented
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

1 participant