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

Please document (and verify via tests) which http_connector/tcp_connector gets used for IMDS (to allow https_only for others) #579

Closed
joshtriplett opened this issue Jul 10, 2022 · 8 comments
Labels
bug This issue is a bug. p3 This is a minor priority issue

Comments

@joshtriplett
Copy link
Contributor

Describe the bug

Setting up a custom connector requires providing that connector in several places: in the ProviderConfig, in the SdkConfig, and in the various API clients.

One of the first two gets used for IMDS, to retrieve things like the region or authentication tokens. Providing a connector that only supports https will break in this case ("Unsupported scheme http").

Please document which of the connectors gets used for IMDS, and that that connector must support HTTP.

Ideally, please also verify via tests that that connector gets used for http to IMDS (and never for https), and other connectors are never used for http, only https.

Expected Behavior

I'd like to get some kind of indication that https_only won't work for one specific connector, and a clear indication of which connector.

Current Behavior

"Unsupported scheme http"

Reproduction Steps

Configure a hyper-rustls connector, calling .https_only() on its builder during setup.

Use that connector in the provider config (via .http_connector) and the SdkConfig (via .http_connector and indirectly via the provider config passed to .configure).

Observe the failure documented above.

Possible Solution

No response

Additional Information/Context

No response

Version

├── aws-config v0.15.0
│   ├── aws-http v0.15.0
│   │   ├── aws-smithy-http v0.45.0
│   │   │   ├── aws-smithy-eventstream v0.45.0
│   │   │   │   ├── aws-smithy-types v0.45.0
│   │   │   ├── aws-smithy-types v0.45.0 (*)
│   │   ├── aws-smithy-types v0.45.0 (*)
│   │   ├── aws-types v0.15.0
│   │   │   ├── aws-smithy-async v0.45.0
│   │   │   ├── aws-smithy-client v0.45.0
│   │   │   │   ├── aws-smithy-async v0.45.0 (*)
│   │   │   │   ├── aws-smithy-http v0.45.0 (*)
│   │   │   │   ├── aws-smithy-http-tower v0.45.0
│   │   │   │   │   ├── aws-smithy-http v0.45.0 (*)
│   │   │   │   ├── aws-smithy-types v0.45.0 (*)
│   │   │   ├── aws-smithy-http v0.45.0 (*)
│   │   │   ├── aws-smithy-types v0.45.0 (*)
│   ├── aws-sdk-sso v0.15.0
│   │   ├── aws-endpoint v0.15.0
│   │   │   ├── aws-smithy-http v0.45.0 (*)
│   │   │   ├── aws-types v0.15.0 (*)
│   │   ├── aws-http v0.15.0 (*)
│   │   ├── aws-sig-auth v0.15.0
│   │   │   ├── aws-sigv4 v0.15.0
│   │   │   │   ├── aws-smithy-eventstream v0.45.0 (*)
│   │   │   │   ├── aws-smithy-http v0.45.0 (*)
│   │   │   ├── aws-smithy-eventstream v0.45.0 (*)
│   │   │   ├── aws-smithy-http v0.45.0 (*)
│   │   │   ├── aws-types v0.15.0 (*)
│   │   ├── aws-smithy-async v0.45.0 (*)
│   │   ├── aws-smithy-client v0.45.0 (*)
│   │   ├── aws-smithy-http v0.45.0 (*)
│   │   ├── aws-smithy-http-tower v0.45.0 (*)
│   │   ├── aws-smithy-json v0.45.0
│   │   │   └── aws-smithy-types v0.45.0 (*)
│   │   ├── aws-smithy-types v0.45.0 (*)
│   │   ├── aws-types v0.15.0 (*)
│   ├── aws-sdk-sts v0.15.0
│   │   ├── aws-endpoint v0.15.0 (*)
│   │   ├── aws-http v0.15.0 (*)
│   │   ├── aws-sig-auth v0.15.0 (*)
│   │   ├── aws-smithy-async v0.45.0 (*)
│   │   ├── aws-smithy-client v0.45.0 (*)
│   │   ├── aws-smithy-http v0.45.0 (*)
│   │   ├── aws-smithy-http-tower v0.45.0 (*)
│   │   ├── aws-smithy-query v0.45.0
│   │   │   ├── aws-smithy-types v0.45.0 (*)
│   │   ├── aws-smithy-types v0.45.0 (*)
│   │   ├── aws-smithy-xml v0.45.0
│   │   ├── aws-types v0.15.0 (*)
│   ├── aws-smithy-async v0.45.0 (*)
│   ├── aws-smithy-client v0.45.0 (*)
│   ├── aws-smithy-http v0.45.0 (*)
│   ├── aws-smithy-http-tower v0.45.0 (*)
│   ├── aws-smithy-json v0.45.0 (*)
│   ├── aws-smithy-types v0.45.0 (*)
│   ├── aws-types v0.15.0 (*)
├── aws-sdk-ec2 v0.15.0
│   ├── aws-endpoint v0.15.0 (*)
│   ├── aws-http v0.15.0 (*)
│   ├── aws-sig-auth v0.15.0 (*)
│   ├── aws-smithy-async v0.45.0 (*)
│   ├── aws-smithy-client v0.45.0 (*)
│   ├── aws-smithy-http v0.45.0 (*)
│   ├── aws-smithy-http-tower v0.45.0 (*)
│   ├── aws-smithy-query v0.45.0 (*)
│   ├── aws-smithy-types v0.45.0 (*)
│   ├── aws-smithy-xml v0.45.0 (*)
│   ├── aws-types v0.15.0 (*)
├── aws-sdk-s3 v0.15.0
│   ├── aws-endpoint v0.15.0 (*)
│   ├── aws-http v0.15.0 (*)
│   ├── aws-sig-auth v0.15.0 (*)
│   ├── aws-sigv4 v0.15.0 (*)
│   ├── aws-smithy-async v0.45.0 (*)
│   ├── aws-smithy-client v0.45.0 (*)
│   ├── aws-smithy-eventstream v0.45.0 (*)
│   ├── aws-smithy-http v0.45.0 (*)
│   ├── aws-smithy-http-tower v0.45.0 (*)
│   ├── aws-smithy-types v0.45.0 (*)
│   ├── aws-smithy-xml v0.45.0 (*)
│   ├── aws-types v0.15.0 (*)
├── aws-sdk-sts v0.15.0 (*)
├── aws-smithy-client v0.45.0 (*)

Environment details (OS name and version, etc.)

Debian, latest sid

Logs

No response

@joshtriplett joshtriplett added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Jul 10, 2022
@joshtriplett
Copy link
Contributor Author

Following up on this: several existing bits of documentation showing how to use ProviderConfig give examples that build a connector with https_only, and those examples will fail for this reason: the connector won't be able to access IMDS.

@Velfi Velfi changed the title Please document (and verify via tests) which http_connector/tcp_connector gets used for IMDB (to allow https_only for others) Please document (and verify via tests) which http_connector/tcp_connector gets used for IMDS (to allow https_only for others) Jul 12, 2022
@Velfi Velfi removed needs-triage This issue or PR still needs to be triaged. labels Jul 14, 2022
@Velfi
Copy link
Contributor

Velfi commented Jul 14, 2022

I'm looking at how we define and use connectors in the SDK right now with the aim of simplifying things. I'll make sure, when the design solidifies, to make it as obvious as possible what can be set, how, and where it will be used.

@joshtriplett
Copy link
Contributor Author

That sounds great. I would love to see a simplified model here. Thank you!

@Velfi
Copy link
Contributor

Velfi commented Jul 18, 2022

The team is not going to be able to address this issue in the short term but we do consider it important and we'll be thinking about it.

@jdisanti jdisanti added the low-priority This issue is low-priority and will be addressed once we have fewer high-priority tasks label Jul 20, 2022
@jmklix jmklix added p3 This is a minor priority issue and removed low-priority This issue is low-priority and will be addressed once we have fewer high-priority tasks labels Nov 14, 2022
@rcoh
Copy link
Contributor

rcoh commented Jul 25, 2023

The right thing to do here is adding an https_only flag to ConnectorSettings. That will allow us to conditionally create the right connectors.

@Velfi
Copy link
Contributor

Velfi commented Aug 3, 2023

Per the announcement in the latest release:

Behavior change: Credential providers now share the HTTP connector used by the SDK. If you want to keep a separate connector for clients, use <service>::ConfigBuilder::http_connector when constructing the client.

@jmklix
Copy link
Member

jmklix commented Mar 15, 2024

This looks like this has been documented in the latest version. Please let us know if you still have any questions/concerns about this.

@jmklix jmklix closed this as completed Mar 15, 2024
Copy link

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue is a bug. p3 This is a minor priority issue
Projects
None yet
Development

No branches or pull requests

5 participants