aws-cloudformation / rain

A development workflow tool for working with AWS CloudFormation.
Apache License 2.0
771 stars 69 forks source link

Added a `cc drift` command that detects and remediates drift #268

Closed ericzbeard closed 6 months ago

ericzbeard commented 6 months ago

This only applies to templates deployed with rain cc deploy, an experimental feature that bypasses CloudFormation stacks and deploys resources directly with Cloud Control API. We might also consider building something like this for normal CloudFormation deployments.

As sample of the UI for detecting drift in a resource:

Screenshot 2024-02-01 at 2 48 49 PM

@hariprakash-j, @khmoryz, @null93, @iainelder, would love some feedback on this. (Still working on approvals so I can add you as actual reviewers)

null93 commented 6 months ago

@ericzbeard I was not aware of the Cloud Control API until today, so this was a good excuse for me to try it out. In general it looks very promising and I definitely see a use case for this type of deployment style (similar to terraform). Since this was my first interaction with the cc command, my feedback is gonna be for all the sub-commands including the drift sub-command. Here are my thoughts:

Display Help Menu For CC Command

I would delete this line so cobra would print out the help screen with the sub-commands. Right now it prints out this:

./bin/rain cc
Usage: cc deploy|rm|state|drift

Add Profile Flag

Got around it for now by prefixing command with AWS_PROFILE=my-profile-name

$ ./bin/rain cc --profile my-profile-name deploy
Error: unknown flag: --profile
Usage:
  rain cc deploy <template> <name>

Flags:
  -c, --config string           YAML or JSON file to set tags and parameters
      --debug                   Output debugging information
  -x, --experimental            Acknowledge that this is an experimental feature
  -h, --help                    help for deploy
      --ignore-unknown-params   Ignore unknown parameters
      --no-colour               Disable colour output
      --params strings          set parameter values; use the format key1=value1,key2=value2
  -s, --state                   Instead of deploying, download the state file
      --tags strings            add tags to the stack; use the format key1=value1,key2=value2
  -u, --unlock string           Unlock <lockid> and continue
  -y, --yes                     don't ask questions; just deploy

What Resources Are Supported?

My first attempt was to create an EC2 instance but It was not supported:

$ AWS_PROFILE=my-profile-name ./bin/rain cc deploy -x ./private/compute.yml tester
Enter a value for parameter 'ImageId' (default value: /aws/service/ami-amazon-linux-latest/amzn2-ami-hvm-x86_64-gp2):
AWS::EC2::Instance is not full supported by CCAPI
Unable to deploy this template due to unsupported resources

I then changed it to an S3 bucket and it worked, but it made me wonder which resources are currently supported?

Didn't Ask Permission To Create Rain Assets Bucket

I noticed that it did not ask me whether it was okay to create an s3 bucket for rain assets. If I remember correctly the regular deploy command does ask to create it beforehand.

Custom S3 Bucket or Store State Locally?

There was no flags to specify a custom bucket or prefix. I am sure this is because this is very experimental, but I thought it was worth noting.

Also, would it be possible to save and reference the state file locally instead of keeping it in an S3 bucket?

Drift Detection Worked Perfectly

I modified the "live" resource and the drift command correctly caught the change and displayed it really nicely.

Hard To Read Drift Detection With No Color Flag

The red color was really helpful to see what drifted. But when passing the --no-colour flag it was hard to parse the differences. This could obviously be a problem for color blind folks or certain setups. Maybe alongside the color change, we can have the differences shown in the standard unified diff format that we are all used to with git?

🔎 MyBucket (AWS::S3::Bucket urxhricfutgviqtv1vuo0go1q-aq2yjgqi6m6y)... Drift detected!

    ========== âš¡ Live state âš¡ ==========
    Arn: arn:aws:s3:::urxhricfutgviqtv1vuo0go1q-aq2yjgqi6m6y
    BucketEncryption:
      ServerSideEncryptionConfiguration:
        [0]:
          BucketKeyEnabled: false
          ServerSideEncryptionByDefault:
            SSEAlgorithm: AES256
    BucketName: urxhricfutgviqtv1vuo0go1q-aq2yjgqi6m6y
    DomainName: urxhricfutgviqtv1vuo0go1q-aq2yjgqi6m6y.s3.amazonaws.com
    DualStackDomainName: urxhricfutgviqtv1vuo0go1q-aq2yjgqi6m6y.s3.dualstack.us-east-1.amazonaws.com
    PublicAccessBlockConfiguration:
      BlockPublicAcls: true
      BlockPublicPolicy: true
      IgnorePublicAcls: true
      RestrictPublicBuckets: true
    RegionalDomainName: urxhricfutgviqtv1vuo0go1q-aq2yjgqi6m6y.s3.us-east-1.amazonaws.com
    WebsiteURL: http://urxhricfutgviqtv1vuo0go1q-aq2yjgqi6m6y.s3-website-us-east-1.amazonaws.com

    ========== 📄 Stored state 📄 ==========
    Arn: arn:aws:s3:::urxhricfutgviqtv1vuo0go1q-aq2yjgqi6m6y
    BucketEncryption:
      ServerSideEncryptionConfiguration:
        [0]:
          BucketKeyEnabled: false
          ServerSideEncryptionByDefault:
            SSEAlgorithm: AES256
    BucketName: urxhricfutgviqtv1vuo0go1q-aq2yjgqi6m6y
    DomainName: urxhricfutgviqtv1vuo0go1q-aq2yjgqi6m6y.s3.amazonaws.com
    DualStackDomainName: urxhricfutgviqtv1vuo0go1q-aq2yjgqi6m6y.s3.dualstack.us-east-1.amazonaws.com
    PublicAccessBlockConfiguration:
      BlockPublicAcls: false
      BlockPublicPolicy: false
      IgnorePublicAcls: false
      RestrictPublicBuckets: false
    RegionalDomainName: urxhricfutgviqtv1vuo0go1q-aq2yjgqi6m6y.s3.us-east-1.amazonaws.com
    WebsiteURL: http://urxhricfutgviqtv1vuo0go1q-aq2yjgqi6m6y.s3-website-us-east-1.amazonaws.com

Conclusion

Overall this was really fun to play with and you did a great job! My feedback might be a little out of scope of this PR, but I hope you got some value from it. I am really looking forward to seeing this feature evolve in the future.

ericzbeard commented 6 months ago

@null93 Amazing feedback, thank you!

Display Help Menu For CC Command

Nice, I didn't know I could do this with Cobra. Much better.

Add Profile Flag

I need to get universal args figured out with Cobra, I thought that was already available everywhere.

What Resources Are Supported?

You can query the API to see what's fully supported. We put a huge effort into improving coverage over the last year, but we still have a few resources left to go. They are mostly the ones that don't get used often.

aws cloudformation list-types --type RESOURCE \
        --provisioning-type NON_PROVISIONABLE \
        --visibility PUBLIC --filters TypeNamePrefix=AWS:: \
         | jq -r ".TypeSummaries" | jq -r ".[] | .TypeName" | wc -l

Hard To Read Drift Detection With No Color Flag

I hadn't considered that. I wanted this to be as human-readable as possible, and I find standard diffs to be brain-teasers. Maybe I could make that an option?

Didn't Ask Permission To Create Rain Assets Bucket

Good catch, I missed that.

null93 commented 6 months ago

Glad I could help!

I need to get universal args figured out with Cobra, I thought that was already available everywhere.

I didn't test this, but I think you would want to replace this line with:

addCommand(stackGroup, true, true, cc.CCDeployCmd)
addCommand(stackGroup, true, true, cc.CCRmCmd)
addCommand(stackGroup, true, true, cc.CCStateCmd)
addCommand(stackGroup, true, true, cc.CCDriftCmd)

You can query the API to see what's fully supported. We put a huge effort into improving coverage over the last year, but we still have a few resources left to go. They are mostly the ones that don't get used often.

Beautiful! Thank you!

I hadn't considered that. I wanted this to be as human-readable as possible, and I find standard diffs to be brain-teasers. Maybe I could make that an option?

Definitely not a big deal for me, just something I noticed.

hariprakash-j commented 6 months ago

hi @ericzbeard , to add to what @null93 said about asking the user to create a s3 bucket, its hard coded as true right now. I think it would be better to have a --yes flag for the cc drift command that would auto create a bucket user uses that flag or explicitly ask the user if they want to create one. This is to avoid any surprises for the user.

the hard coding i mentioned is in L37 of the drift.go file

a bit out of scope topic, maybe we can add a global flag --no-confirm similar to what cdk has and use that to deal with bucket creations everywhere, I've noticed that bucket creation is inconsistent across rain, I think stacksets does not have the --force option , not sure about this. This global flag could be a good way to handle this regardless.

Edit: stacksets has the --yes flag to force create bucket and not ask confirmation for deployment

ericzbeard commented 6 months ago

hi @ericzbeard , to add to what @null93 said about asking the user to create a s3 bucket, its hard coded as true right now. I think it would be better to have a --yes flag for the cc drift command that would auto create a bucket user uses that flag or explicitly ask the user if they want to create one. This is to avoid any surprises for the user.

I fixed that in a later commit, along with most of the other suggestions. It should honor the --yes command now. I also incorporated drift remediation into the cc deploy command when there is already a state file.

ericzbeard commented 6 months ago

(Still working on approvals so I can add you as actual reviewers)

This is done! Invites to the repo sent.

ericzbeard commented 6 months ago

Tested everything out and it looks good! The only thing that I caught that did not work as expected was the --s3-prefix flag. I ran this:

./bin/rain cc deploy test.yaml tester -x --profile my-profile --s3-bucket my-bucket --s3-prefix test-cc

And the state file saved to /deployments/tester.yaml instead of /test-cc/deployments/tester.yaml

Fixed