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

Abstract AWS Vault for cmd binaries #2114

Merged
merged 29 commits into from May 17, 2019

Conversation

2 participants
@chrisgilmerproj
Copy link
Contributor

commented May 9, 2019

Description

The code for compare-secure-migrations doesn't have our nice built-in aws-vault integration. In this PR I've broken out the aws-vault command line flags and code for setting up an AWS Config struct. I then modified both compare-secure-migrations and ecs-deploy-task-container to use that code.

In addition I applied the new AWS region stuff to milmove ses and s3 command line flags. I also updated the way we manage tests to ensure specific flags won't fail when running server tests.

I did not modify ecs-ervice-logs because we intend to open source that at some point in the future.

Setup

make bin/compare-secure-migrations
compare-secure-migrations

Code Review Verification Steps

  • Tested in the Experimental environment (for changes to containers, app startup, or connection to data stores)
  • Request review from a member of a different team.
  • Have the Pivotal acceptance criteria been met for this change?

References

@chrisgilmerproj chrisgilmerproj self-assigned this May 9, 2019

@codecov

This comment has been minimized.

Copy link

commented May 10, 2019

Codecov Report

Merging #2114 into master will decrease coverage by 0.2%.
The diff coverage is 21.74%.

@@            Coverage Diff            @@
##           master    #2114     +/-   ##
=========================================
- Coverage   58.98%   58.78%   -0.2%     
=========================================
  Files         238      240      +2     
  Lines       13807    13888     +81     
=========================================
+ Hits         8144     8164     +20     
- Misses       4673     4728     +55     
- Partials      990      996      +6

@chrisgilmerproj chrisgilmerproj marked this pull request as ready for review May 10, 2019

Show resolved Hide resolved Makefile

@pjdufour-truss pjdufour-truss self-requested a review May 13, 2019

@pjdufour-truss
Copy link
Contributor

left a comment

Please remove errInvalidRegion from cmd/compare-secure-migrations/main.go since it is no longer used.

Show resolved Hide resolved pkg/cli/certs_test.go Outdated

@chrisgilmerproj chrisgilmerproj requested a review from pjdufour-truss May 13, 2019

Show resolved Hide resolved pkg/cli/aws_test.go Outdated

@chrisgilmerproj chrisgilmerproj requested a review from pjdufour-truss May 14, 2019

@pjdufour-truss
Copy link
Contributor

left a comment

Approved, but see minor comment.

chrisgilmerproj added some commits May 16, 2019

Revert "Deploy to experimental"
This reverts commit ef3c94a.
@chrisgilmerproj

This comment has been minimized.

Copy link
Contributor Author

commented May 16, 2019

Deploying to experimental revealed a couple things I'd overlooked. Fixes are in and the last deploy worked! This is ready to merge once the tests all pass.

chrisgilmerproj added some commits May 17, 2019

AWS vault has some defaults which should be respected in our applicat…
…ion as well as us needing to check if keychain name and aws profile are set.

@chrisgilmerproj chrisgilmerproj merged commit 1359d79 into master May 17, 2019

18 of 20 checks passed

codecov/patch 21.74% of diff hit (target 58.98%)
Details
codecov/project/go 58.6% (-0.2%) compared to 3b72a74
Details
Codacy/PR Quality Review Up to standards. A positive pull request.
Details
ci/circleci: acceptance_tests_experimental Your tests passed on CircleCI!
Details
ci/circleci: acceptance_tests_local Your tests passed on CircleCI!
Details
ci/circleci: acceptance_tests_staging Your tests passed on CircleCI!
Details
ci/circleci: build_app Your tests passed on CircleCI!
Details
ci/circleci: build_migrations Your tests passed on CircleCI!
Details
ci/circleci: build_tasks Your tests passed on CircleCI!
Details
ci/circleci: build_tools Your tests passed on CircleCI!
Details
ci/circleci: client_test Your tests passed on CircleCI!
Details
ci/circleci: integration_tests_api Your tests passed on CircleCI!
Details
ci/circleci: integration_tests_mymove Your tests passed on CircleCI!
Details
ci/circleci: integration_tests_office Your tests passed on CircleCI!
Details
ci/circleci: integration_tests_tsp Your tests passed on CircleCI!
Details
ci/circleci: pre_deps_golang Your tests passed on CircleCI!
Details
ci/circleci: pre_deps_yarn Your tests passed on CircleCI!
Details
ci/circleci: pre_test Your tests passed on CircleCI!
Details
ci/circleci: server_test Your tests passed on CircleCI!
Details
ci/circleci: server_test_coverage Your tests passed on CircleCI!
Details

@chrisgilmerproj chrisgilmerproj deleted the cg_165642614_add_vault_to_compare_script branch May 17, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.