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

Does not assume role correctly when use salt-cloud to create AWS ec2 #52501

Open
huxiaobabaer opened this issue Apr 11, 2019 · 11 comments
Open
Labels
Bug broken, incorrect, or confusing behavior Confirmed Salt engineer has confirmed bug/feature - often including a MCVE severity-medium 3rd level, incorrect or bad functionality, confusing and lacks a work around
Milestone

Comments

@huxiaobabaer
Copy link

huxiaobabaer commented Apr 11, 2019

Description of Issue/Question

When i try to use salt-cloud to create AWS ec2, it does not assume role correctly

Setup

(Please provide relevant configs and/or SLS files (Be sure to remove sensitive info).)
My purpose is using salt-cloud ec2 in account 22222222 to create AWS ec2 in account 11111111

  • Create role alt-cloud-cross-account in AWS account 11111111 with trust entities = aws account 22222222

  • Attached Full EC2Acess policy to role salt-clou1d-cross-account

  • Create ec2 in AWS account 22222222 and install salt cloud version 2019.2

  • Create ec2 role with policy allow it to assume 11111111:role/salt-cloud-cross-account

  • Config prodiver file:
    xxxxx-xxxxx-xxxx:
    id: 'use-instance-role-credentials'
    key: 'use-instance-role-credentials'
    role_arn: 'arn:aws:iam::11111111:role/salt-cloud-cross-account'
    driver: ec2

  • config provider fine

Steps to Reproduce Issue

(Include debug logs if possible and relevant.)

Run salt-cloud -m test-map-file

From cloudTrail, i can see

  • an api call DescrbeInstance in account 11111111. No instance found.
  • an api call RunInstance in account 11111111. EC2 created in account 11111111
  • seven an api call DescrbeInstance in account 22222222
    return 400, Bad request, an instance with instance id xxx not found
    Salt-Cloud error out

According to cloudTrail, I believe it assumes the role as expected when creating ec2, but after then when it queries the result of the creation, it does not assume role correctly. As it considers ec2 is not created, exit without finishing other following tasks like tag ec2, ssh ec2, install/config/start salt minion

Is it a bug of cloud-salt??

Versions Report

(Provided by running salt --versions-report. Please also mention any differences in master/minion versions.)
2019.2

@Ch3LL
Copy link
Contributor

Ch3LL commented Apr 11, 2019

ping @saltstack/team-cloud any ideas here?

@Ch3LL Ch3LL added the Pending-Discussion The issue or pull request needs more discussion before it can be closed or merged label Apr 11, 2019
@Ch3LL Ch3LL added this to the Blocked milestone Apr 11, 2019
@gtmanfred
Copy link
Contributor

I do not believe we have any code in place for assuming roles. This would require the feature to be added.

@gtmanfred
Copy link
Contributor

Oh, looks like it actually should be assuming the role_arn. I don't know. probably a bug.

@huxiaobabaer
Copy link
Author

huxiaobabaer commented Apr 12, 2019

Could you please reproduce it on your end and confirm if it is a bug?
Thanks

@changyong
Copy link

changyong commented Apr 17, 2019

Hi, I checked the codes and found the problem. When the variable __Expiration__ isn't equal to '' , it will always return from the function directly, then it won't have chance to execute provider.get('role_arn') to get credential from assume role again.

@Ch3LL Ch3LL added Bug broken, incorrect, or confusing behavior fixed-pls-verify fix is linked, bug author to confirm fix severity-medium 3rd level, incorrect or bad functionality, confusing and lacks a work around P3 Priority 3 and removed Pending-Discussion The issue or pull request needs more discussion before it can be closed or merged labels May 2, 2019
@Ch3LL Ch3LL modified the milestones: Blocked, Approved May 2, 2019
@nnsense
Copy link

nnsense commented Nov 17, 2019

@Ch3LL Hi, I'm facing exactly the same issue while trying to create an instance (it doesn't find the security group and fails) and while destroying an instance (it cannot find the instance and fails).
Both commands can initially find the object but on the second call they fail.
Is there a patch I can apply myself? Any ETA for the fix to be implemented? Managing an infrastructure using hardcoded keys is not ideal, besides security concerns we're required to create API keys on every account. Thank you!

@nnsense
Copy link

nnsense commented Nov 17, 2019

Thanks to @changyong comment I've sorted it by changing

if __Expiration__ != '':

into

if __Expiration__ == '':

in utils/aws.py, it works now. I don't mind fetching credential again.

@stale
Copy link

stale bot commented Jan 7, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

If this issue is closed prematurely, please leave a comment and we will gladly reopen the issue.

@stale stale bot added the stale label Jan 7, 2020
@waynew waynew added the Confirmed Salt engineer has confirmed bug/feature - often including a MCVE label Jan 8, 2020
@stale
Copy link

stale bot commented Jan 8, 2020

Thank you for updating this issue. It is no longer marked as stale.

@stale stale bot removed the stale label Jan 8, 2020
@major0
Copy link
Contributor

major0 commented Mar 4, 2021

I think this was closed prematurely. The bug still exists and the fix from #52572 was never merged in.

@major0
Copy link
Contributor

major0 commented Mar 4, 2021

Thanks to @changyong comment I've sorted it by changing

if __Expiration__ != '':

into

if __Expiration__ == '':

in utils/aws.py, it works now. I don't mind fetching credential again.

This was a fairly useful stop-gap to work around this particular bug.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug broken, incorrect, or confusing behavior Confirmed Salt engineer has confirmed bug/feature - often including a MCVE severity-medium 3rd level, incorrect or bad functionality, confusing and lacks a work around
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants