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

Add boto3action to aws pack #37

Closed
wants to merge 11 commits into from

Conversation

xydinesh
Copy link

@xydinesh xydinesh commented Jul 15, 2017

I create this pull request as a result of discussion happened in, StackStorm-Exchange/exchange-incubator#10. Main features in this pull request are,

  • Ability to assume_role for cross account access
  • Ability to specify region for cross region access

For example, I have a st2 deployment in aws account 123456 and us-east-1. I want to deploy VPC in account 456789 and us-west-1.

     assume_role:
        action: aws.assume_role
        input:
           role_arn: “arn:aws:iam:456789:role/st2_role”
        publish:
          credentials: <% task(assume_role).result.result %>
       on-success:
         - create_vpc
    
    create_vpc:
      action: aws.boto3action
      input:
        service: ec2
        action_name: create_vpc
        region: "us-west-1"
        params: <% dict(CidrBlock => "10.0.0.0/16", InstanceTenancy => "default") %>
        credentials: <% $.credentials %>
      publish:
          vpc_id: <% task(create_vpc).result.result.Vpc.VpcId %>
      on-success:
        - create_subnet

    create_subnet:
      action: aws.boto3action
      input:
        service: ec2
        region: "us-west-1"
        action_name: create_subnet
        params: <% dict(AvailabilityZone =>"us-west-1a", CidrBlock =>"10.0.0.0/24", VpcId => $.vpc_id) %>
        credentials: <% $.credentials %>
      publish:
        subnet_ids: <% task(create_subnets).result.result.Subnet.SubnetId %>
      on-success:
         - create_igw

     create_igw:
        action: aws.boto3action
        input:
          service: ec2
          action_name: create_internet_gateway
          region: <% $.region %>
          credentials: <% $.credentials %>
        publish:
          igw_id: <% task(create_igw).result.result.InternetGateway.InternetGatewayId %>

In addition, aws.boto3action created with following opinions.

  • aws credentials
    Boto3 is the official SDK for AWS. As a user/developer, If I have boto3 configured I expect aws pack to work without doing any additional configuration. For example,
st2 pack install aws
st2 run aws.boto3action service="ec2" action_name="decribe_vpcs" region="us-west-1"

In addition, if I want to use use boto3 profiles

st2 run aws.boto3action service="ec2" action_name="decribe_vpcs" region="us-west-1" env="AWS_PROFILE=production"
  • yaml generation
    Long term, I don’t believe yaml generation scale based on the number of services AWS have and introduce. In addition, IMO Boto3 documentation is detailed, has examples. Having yaml for each action is redundant and add no value to the end user.

  • pack maintenance

    Since there are no yaml to generate, this pack should be easy to maintain. Any new service boto3 introduce, available to pack user right away.

@Mierdin
Copy link

Mierdin commented Jul 15, 2017

Thanks for opening this - might be worth including a link to https://github.com/StackStorm-Exchange/exchange-incubator, as there was some useful context captured there. Summarizing the intention in the description would be good too. I have a feeling the future of this pack will refer back to this PR at least a few times 😄

@xydinesh
Copy link
Author

@Mierdin Good call. I brought over, what I thought important points to description. Please see if I am missing anything.

@warrenvw
Copy link
Contributor

warrenvw commented Jul 17, 2017

Thanks for the PR.

Per our conversation, I'd suggest updating the README with examples on how to use the new actions. Document examples with and without using assume_role.

@AndyMoore
Copy link
Contributor

as discussed with Dinesh the main issue i have with this approach is it stops actions being discreet and independent - you'd need to call the assume_role action each time before any other action, which means to run any action against a second account will force each action to become a workflow.

The model I'd prefer is to have the assume_role and region as arguments within each action, which would then be passed through run.py and lib/action.py so each action can still be called individually

have raised #38 to show this approach. Haven't regenerated the actions in the pack though to make it easier to see the actual change

@xydinesh
Copy link
Author

Andy, case you are making is useful for testing and debugging cross account calls. I agree that we should consider adding assume_role to boto3action, to make it independent. However, in a workflow, I don't think it is make sense to call assume_role for every action. In a workflow, I prefer to assume_role once and reuse credentials in actions.

@AndyMoore
Copy link
Contributor

the difference is the action run at the beginning to get credentials, and everything else relying on that being there, as opposed to all actions standing on their own and being discrete

you either have to update every action to be able to take credentials, which i'd prefer not to pass between actions - that's managed on a pack level imo - or you're updating every action to be able to assume a role. both require every action to be updated (via the generator), but its whether you allow a user to enter credentials, or whether you let stackstorm/aws pack look after credentials under the hood instead. obviously both could be added and maybe that's the way that allows the most flexibility, but then you have a large generated pack with one single non-generated action

one for the stackstorm guys as it's not really a technical question @warrenvw

@xydinesh
Copy link
Author

xydinesh commented Aug 7, 2017

Should we close this ? As we have same PR against boto3 branch.

@warrenvw
Copy link
Contributor

warrenvw commented Aug 8, 2017

Closing. This PR is replaced by #44.

@warrenvw warrenvw closed this Aug 8, 2017
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.

4 participants