Skip to content

Resolves #19790, Provides Support for IAM Credentials #20891

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

Merged
merged 1 commit into from
Aug 20, 2020
Merged

Resolves #19790, Provides Support for IAM Credentials #20891

merged 1 commit into from
Aug 20, 2020

Conversation

cuppett
Copy link
Contributor

@cuppett cuppett commented May 9, 2020

Includes support for either leveraging environment variables passed to the PHP runtime or IAM instance profile present on the host being used. The default and first choice is still the parameter file as documented.

See also: Developer Guide -> Chaining Providers

@kesselb kesselb linked an issue May 9, 2020 that may be closed by this pull request
@kesselb kesselb added 3. to review Waiting for reviews enhancement labels May 9, 2020
@kesselb kesselb added this to the Nextcloud 20 milestone May 9, 2020
@kesselb kesselb requested review from icewind1991 and rullzer May 9, 2020 15:45
Copy link
Contributor

@kesselb kesselb left a comment

Choose a reason for hiding this comment

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

Thanks 👍

@LordEhlegant LordEhlegant mentioned this pull request Jun 4, 2020
@cuppett
Copy link
Contributor Author

cuppett commented Jul 5, 2020

Anything else needed from me here? Is there a way for me to re-launch/attempt the failing CI build (or is it a common failure)?

@kesselb
Copy link
Contributor

kesselb commented Jul 6, 2020

Anything else needed from me here

I don't think so.

for me to re-launch/attempt the failing CI build

Rebase would be nice.

is it a common failure

Yes.

@cuppett
Copy link
Contributor Author

cuppett commented Jul 6, 2020

Anything else needed from me here

I don't think so.

for me to re-launch/attempt the failing CI build

Rebase would be nice.

is it a common failure

Yes.

Thanks! Rebase done.

@kesselb kesselb requested a review from MorrisJobke July 6, 2020 11:22
@MorrisJobke
Copy link
Member

Rebased because we had some broken test on master today. Sorry for that.

@MorrisJobke MorrisJobke mentioned this pull request Aug 11, 2020
57 tasks
Copy link
Member

@MorrisJobke MorrisJobke left a comment

Choose a reason for hiding this comment

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

Code looks good 👍

@MorrisJobke
Copy link
Member

Let me rebase again and @rullzer will review later today.

Includes support for either leveraging environment variables
passed to the PHP runtime or IAM instance profile present
on the host being used. The default and first choice is
still the parameter file as documented.

See also: https://docs.aws.amazon.com/sdk-for-php/v3/developer-guide/guide_credentials_provider.html#chaining-providers

Signed-off-by: Stephen Cuppett <[email protected]>
@kesselb
Copy link
Contributor

kesselb commented Aug 20, 2020

Psalm warnings looks like a false positive to me. Should work if we import the function use function GuzzleHttp\Promise\promise_for; and use it like

				return promise_for(
					new Credentials($key, $secret)
				);

@cuppett
Copy link
Contributor Author

cuppett commented Aug 20, 2020

Psalm warnings looks like a false positive to me. Should work if we import the function use function GuzzleHttp\Promise\promise_for; and use it like

				return promise_for(
					new Credentials($key, $secret)
				);

I have that cleanup... squash and force push or add commit?

Copy link
Member

@rullzer rullzer left a comment

Choose a reason for hiding this comment

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

Still works when running with minio!
Good stuff!

@rullzer rullzer merged commit 6e4b089 into nextcloud:master Aug 20, 2020
@welcome
Copy link

welcome bot commented Aug 20, 2020

Thanks for your first pull request and welcome to the community! Feel free to keep them coming! If you are looking for issues to tackle then have a look at this selection: https://github.com/nextcloud/server/issues?q=is%3Aopen+is%3Aissue+label%3A%22good+first+issue%22

@cuppett cuppett deleted the cuppett/issue#19790 branch August 20, 2020 18:34
@MorrisJobke
Copy link
Member

MorrisJobke commented Aug 20, 2020

I have that cleanup... squash and force push or add commit?

Both are fine ... @cuppett mind to send another PR ... it seems @rullzer merged too early 🙈

@MorrisJobke
Copy link
Member

Psalm warnings looks like a false positive to me. Should work if we import the function use function GuzzleHttp\Promise\promise_for; and use it like

				return promise_for(
					new Credentials($key, $secret)
				);

Tested locally and both result in the problem. Let me add it to the warning list for now.

@MorrisJobke MorrisJobke mentioned this pull request Aug 20, 2020
@MorrisJobke
Copy link
Member

#22345

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enhance S3 to support EC2 instance roles
4 participants