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

Shutdown callback support for all credentials providers #26

Merged
merged 18 commits into from
Nov 21, 2019

Conversation

bretambrose
Copy link
Contributor

@bretambrose bretambrose commented Sep 13, 2019

  • Shutdown callbacks for credentials providers can be configured from creation options structs; callback is made when all the provider's async resources are no longer in use (client bootstrap atm)
  • aws_signer is removed and is conceptually a function pointer; all configurable signing state should go in a signing_config variant
  • aws_signing_config_aws is now copied inside the sigv4 signing state and dynamically allocated state is copied as well; the passed in config does not need to persist for the duration of the signing process

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@bretambrose bretambrose changed the title First draft Bootstrap released callbacks for credentials providers that used connection manager Sep 13, 2019
@bretambrose bretambrose changed the title Bootstrap released callbacks for credentials providers that used connection manager Shutdown callback support for all credentials providers Nov 8, 2019
Copy link
Contributor

@justinboswell justinboswell left a comment

Choose a reason for hiding this comment

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

LGTM, but one of the more auth-immersed folks should double check

struct aws_byte_cursor access_key_id,
struct aws_byte_cursor secret_access_key,
struct aws_byte_cursor session_token);
struct aws_credentials_provider_static_options *options);
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: const struct *options

*/
AWS_AUTH_API
struct aws_signer *aws_signer_new_aws(struct aws_allocator *allocator);
int aws_sign_request_aws(
Copy link
Contributor

Choose a reason for hiding this comment

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

Worth having a typedef to cover all signers?
typedef int aws_signing_fn(.....);

int aws_sign_request_aws(
struct aws_allocator *allocator,
const struct aws_signable *signable,
const struct aws_signing_config_base *base_config,
Copy link
Contributor

Choose a reason for hiding this comment

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

This can be aws_signing_config_aws now!

source/aws_signing.c Outdated Show resolved Hide resolved
Comment on lines 52 to 53
if (provider) {
if (provider->shutdown_options.shutdown_callback) {
Copy link
Contributor

Choose a reason for hiding this comment

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

[nit]

if (provider && provider->shutdown_options.shutdown_callback) {

@bretambrose bretambrose merged commit 57ba3ae into master Nov 21, 2019
@bretambrose bretambrose deleted the ImdsBootstrapRelease branch January 28, 2020 19:43
rccarper pushed a commit that referenced this pull request Feb 7, 2020
* log errors in decoder
* slight modifications to decoder, while we're in here thinking about what might go wrong.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants