etcd-operator icon indicating copy to clipboard operation
etcd-operator copied to clipboard

Add support to use IAM roles without credentials on s3 backup and restore

Open slok opened this issue 7 years ago • 12 comments

Fixes #1682

This PR allows not using credentials and use instead node IAM roles or applications like Kiam or Kube2iam.

  • Add new S3 client without credentials if not awsSecret present.
  • The region when not using awsSecret will be inferred from the S3 endpoint.
  • Add documentation and example of how to use based on IAM roles.
  • Format some parts of the docs markdown.

I added Minio-go dependency only to infer the region, I thought it would be cleaner and had fewer corner cases/errors than implementing myself as it has a little bit of logic depending on the kind of regions/cases.

slok avatar Dec 06 '18 16:12 slok

Can one of the admins verify this patch?

etcd-bot avatar Dec 06 '18 16:12 etcd-bot

Can one of the admins verify this patch?

etcd-bot avatar Dec 06 '18 16:12 etcd-bot

Can one of the admins verify this patch?

etcd-bot avatar Dec 06 '18 16:12 etcd-bot

@etcd-bot ok to test

hexfusion avatar Dec 06 '18 17:12 hexfusion

I didn't know that this app was used like (or affects as) a library so we mark the internal changes as breaking. Sorry about that. I'll add to the changelog!

slok avatar Dec 07 '18 07:12 slok

@hexfusion ping

raoofm avatar Jan 08 '19 16:01 raoofm

@raoofm I think this PR needs some thought adding this as is would not be optimal. Let me try to get some cycles on this soon, thanks for the ping.

hexfusion avatar Jan 08 '19 16:01 hexfusion

@hexfusion From my pov IAM role based access is much more typical than using keys and secrets. That said the auth config for the go sdk is well defined and IAM is documented as preferred https://docs.aws.amazon.com/sdk-for-go/v1/developer-guide/configuring-sdk.html.

This code is already a bit odd with the requirement to create a shared config file as a secret and then write that out to the fs when it isn't one of the default credential providers for the go sdk.

Here is one proposal to help push this PR forward:

  • Do the file management stuff automatically if the optional secret is set
  • Create a custom credential chain that includes the shared config file, credential file and IAM provider. (I think this is needed since the shared config file is not in the default credential provider, alternatively if shared config is dropped then this isn't needed)
  • Create the S3 client from the custom credential provider (similarly the default chain provider could be used if shared config is dropped)

Doing it this way delegates to the SDK and means a custom Auth object isn't needed?

danbeaulieu avatar Jan 08 '19 22:01 danbeaulieu

Does anyone have ETA on this? I need to setup backups and really don't want to create a user account with keys just to do that...

Thanks!

yzargari avatar Jun 04 '19 13:06 yzargari

Hey guys any progress on this?.

jescarri avatar Jun 13 '19 20:06 jescarri

Hey guys any help required here to get this merged?

I've been running this PR + Prometheus exporter one in prod for a month and it's been working w/o problems.

Br3wm4ster avatar Jul 12 '19 04:07 Br3wm4ster

@Br3wm4ster We have also been running this fork for the backups for several months (since the PR) and we didn't have any problems neither.

Apparently, this seems to be blocked/stopped because it wasn't optimal (although optimal is a matter of opinion, like perfection or software design, I guess) and we are waiting so the maintainers have some spare cycles to revisit this.

By the way, I can help fixing the things that are needed so we can have this feature on master or solving the conflicts with master at this moment.

slok avatar Jul 12 '19 10:07 slok