-
-
Notifications
You must be signed in to change notification settings - Fork 356
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 acmpa ✨ #201
Add acmpa ✨ #201
Changes from 7 commits
5287cde
1ee809e
92e6245
4209721
8f9b7df
d678810
542ee79
0a6016a
2f018d9
f0b78f6
97d6cac
f028ccd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,120 @@ | ||
package aws | ||
|
||
import ( | ||
"sync" | ||
"time" | ||
|
||
"github.com/aws/aws-sdk-go/aws" | ||
"github.com/aws/aws-sdk-go/aws/session" | ||
"github.com/aws/aws-sdk-go/service/acmpca" | ||
"github.com/gruntwork-io/cloud-nuke/config" | ||
"github.com/gruntwork-io/cloud-nuke/logging" | ||
"github.com/gruntwork-io/go-commons/errors" | ||
"github.com/hashicorp/go-multierror" | ||
) | ||
|
||
// getAllACMPCA returns a list of all arns of ACMPCA, which can be deleted. | ||
func getAllACMPCA(session *session.Session, region string, excludeAfter time.Time, configObj config.Config) ([]*string, error) { | ||
svc := acmpca.New(session) | ||
var arns []*string | ||
if paginationErr := svc.ListCertificateAuthoritiesPages(&acmpca.ListCertificateAuthoritiesInput{}, func(p *acmpca.ListCertificateAuthoritiesOutput, lastPage bool) bool { | ||
for _, ca := range p.CertificateAuthorities { | ||
if shouldIncludeACMPCA(ca, excludeAfter, configObj) { | ||
arns = append(arns, ca.Arn) | ||
} | ||
} | ||
return !lastPage | ||
}); paginationErr != nil { | ||
return nil, errors.WithStackTrace(paginationErr) | ||
} | ||
return arns, nil | ||
} | ||
|
||
func shouldIncludeACMPCA(ca *acmpca.CertificateAuthority, excludeAfter time.Time, configObj config.Config) bool { | ||
if ca == nil { | ||
return false | ||
} | ||
|
||
// one can only delete CAs if they are 'ACTIVE' or 'DISABLED' | ||
statusSafe := aws.StringValue(ca.Status) | ||
isCandidateForDeletion := statusSafe == acmpca.CertificateAuthorityStatusActive || statusSafe == acmpca.CertificateAuthorityStatusDisabled | ||
if !isCandidateForDeletion { | ||
return false | ||
} | ||
|
||
// reference time for excludeAfter is lastStateChangeAt time, | ||
// unless it was never changed and createAt time is used. | ||
var referenceTime time.Time | ||
if ca.LastStateChangeAt == nil { | ||
referenceTime = aws.TimeValue(ca.CreatedAt) | ||
} else { | ||
referenceTime = aws.TimeValue(ca.LastStateChangeAt) | ||
} | ||
if excludeAfter.Before(referenceTime) { | ||
return false | ||
} | ||
|
||
return config.ShouldInclude( | ||
aws.StringValue(ca.Arn), | ||
configObj.ACMPCA.IncludeRule.NamesRegExp, | ||
configObj.ACMPCA.ExcludeRule.NamesRegExp, | ||
) | ||
} | ||
|
||
// nukeAllACMPCA will delete all ACMPCA, which are given by a list of arns. | ||
func nukeAllACMPCA(session *session.Session, arns []*string) error { | ||
if len(arns) == 0 { | ||
logging.Logger.Infof("No ACMPCA to nuke in region %s", *session.Config.Region) | ||
return nil | ||
} | ||
svc := acmpca.New(session) | ||
|
||
logging.Logger.Infof("Deleting all ACMPCA in region %s", *session.Config.Region) | ||
// There is no bulk delete acmpca API, so we delete the batch of ARNs concurrently using go routines. | ||
wg := new(sync.WaitGroup) | ||
wg.Add(len(arns)) | ||
errChans := make([]chan error, len(arns)) | ||
for i, arn := range arns { | ||
errChans[i] = make(chan error, 1) | ||
go deleteACMPCAASync(wg, errChans[i], svc, arn, aws.StringValue(session.Config.Region)) | ||
} | ||
wg.Wait() | ||
|
||
// Collect all the errors from the async delete calls into a single error struct. | ||
var allErrs *multierror.Error | ||
for _, errChan := range errChans { | ||
if err := <-errChan; err != nil { | ||
allErrs = multierror.Append(allErrs, err) | ||
logging.Logger.Errorf("[Failed] %s", err) | ||
} | ||
} | ||
return errors.WithStackTrace(allErrs.ErrorOrNil()) | ||
} | ||
|
||
// deleteACMPCAASync deletes the provided ACMPCA arn. Intended to be run in a goroutine, using wait groups | ||
// and a return channel for errors. | ||
func deleteACMPCAASync(wg *sync.WaitGroup, errChan chan error, svc *acmpca.ACMPCA, arn *string, region string) { | ||
defer wg.Done() | ||
|
||
logging.Logger.Infof("Setting status to 'DISABLED' for ACMPCA %s in region %s", *arn, region) | ||
if _, updateStatusErr := svc.UpdateCertificateAuthority(&acmpca.UpdateCertificateAuthorityInput{ | ||
CertificateAuthorityArn: arn, | ||
Status: aws.String(acmpca.CertificateAuthorityStatusDisabled), | ||
}); updateStatusErr != nil { | ||
errChan <- updateStatusErr | ||
return | ||
} | ||
logging.Logger.Infof("Did set status to 'DISABLED' for ACMPCA: %s in region %s", *arn, region) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this step necessary? I just tried the test and it failed with:
The PCA was in state There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh. Sorry. I will make the listing more smart and test it There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See the latest commits. Now the tests work as expected. I had to fix a race-condition, where the CA was in a "CREATING" state during test, which one could not see during the test. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is fixed here. Just a side-note how the terraform-provider-aws does it: |
||
|
||
if _, deleteErr := svc.DeleteCertificateAuthority(&acmpca.DeleteCertificateAuthorityInput{ | ||
CertificateAuthorityArn: arn, | ||
// the range is 7 to 30 days. | ||
// since cloud-nuke should not be used in production, | ||
// we assume that the minimum (7 days) is fine. | ||
PermanentDeletionTimeInDays: aws.Int64(7), | ||
}); deleteErr != nil { | ||
errChan <- deleteErr | ||
return | ||
} | ||
logging.Logger.Infof("Deleted ACMPCA: %s", *arn) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,126 @@ | ||
package aws | ||
|
||
import ( | ||
"os" | ||
"testing" | ||
"time" | ||
|
||
awsgo "github.com/aws/aws-sdk-go/aws" | ||
"github.com/aws/aws-sdk-go/aws/session" | ||
"github.com/aws/aws-sdk-go/service/acmpca" | ||
"github.com/gruntwork-io/cloud-nuke/config" | ||
"github.com/gruntwork-io/cloud-nuke/util" | ||
"github.com/gruntwork-io/go-commons/errors" | ||
"github.com/stretchr/testify/assert" | ||
) | ||
|
||
// enableACMPCAExpensiveEnv is used to control whether to run | ||
// the following test or not. The idea is that the test are disabled | ||
// per default and one has to opt-in to enable the test as creating | ||
// and destroying a ACM PCA is expensive. | ||
// Upper bound, worst case: $400 / month per single CA create/delete. | ||
const enableACMPCAExpensiveEnv = "TEST_ACMPCA_EXPENSIVE_ENABLE" | ||
|
||
// runOrSkip decides whether to run or skip the test depending | ||
// whether the env-var `TEST_ACMPCA_EXPENSIVE_ENABLE` is set or not. | ||
func runOrSkip(t *testing.T) { | ||
if _, isSet := os.LookupEnv(enableACMPCAExpensiveEnv); !isSet { | ||
t.Skipf("Skipping the integration test for acmpca. Set the env-var '%s' to enable this expensive test.", enableACMPCAExpensiveEnv) | ||
} | ||
} | ||
|
||
// createTestACMPCA will create am ACMPCA and return its ARN. | ||
func createTestACMPCA(t *testing.T, session *session.Session, name string) *string { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sanity check: Do you have a sense on how much it would cost to run this test? It appears that the cost of ACM Private CA is $400 / month, pro rated to the number of X (days or hours?) that it is active. Does that active period include the days it is waiting to be deleted (the 7 day minimum)? If so, that can rack up quickly for us if we are running cloud nuke tests for each PR. Depending on how much it costs, can you add an environment variable flag so that we can control when we run the tests related to the ACM PCA functionality (and not on every PR)? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am currently talking to our AWS Enterprise contact as indeed their documentation and support is not clear on this and racked up a significant amount of money just for starting/stopping a CA. I will add some environment variable and this test will be opt-in in all cases so you have to make a concious decision to run it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We heard back from our contact. The billing is pro-rated and does reconciliation after 24-48 hours and you only pay for those hours. Hypothesis from Support-Agent
Answer from AWS PCA Service Team confirming hypothesis from Support-Agent aboveQuote
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So how do we think about the pricing here? If we have this test case and it runs, say, nightly for a month, what sort of fee are we talking? What if we get 20 PRs in this repo, and each one does 20 test runs in a month? I just want to be extra sure we don't end up with a massive AWS bill as a surprise from adding this new functionality! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @brikis98 I think we can go with this for now. @weitzjdevk implemented a feature flag to disable the tests by default. We can merge with that disabled. Separately, I'll run an experiment to enable that test locally once. Then, we can check how much our bill was for August and make our decision on whether to continually test it or not based on that bill cost. Does that seem reasonable? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
tl;dr: Price per run approx: Monthly charge (for CA only): Price (CA) per hour approx. Therefore roughly: Price per run Timeline (when starting the first day of the month)
When you do the run in the middle of the month
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have confirmed this cost in our account as well. I think 0.60 per run is a tad bit expensive to be continuously running the tests, but selectively running it everytime this feature changes seems reasonable, so the env var based feature switch is satisfactory. |
||
// As an additional safety guard, we are adding another check here | ||
// to decide whether to run the test or not. | ||
runOrSkip(t) | ||
|
||
svc := acmpca.New(session) | ||
ca, err := svc.CreateCertificateAuthority(&acmpca.CreateCertificateAuthorityInput{ | ||
CertificateAuthorityConfiguration: &acmpca.CertificateAuthorityConfiguration{ | ||
KeyAlgorithm: awsgo.String(acmpca.KeyAlgorithmRsa2048), | ||
SigningAlgorithm: awsgo.String(acmpca.SigningAlgorithmSha256withrsa), | ||
Subject: &acmpca.ASN1Subject{ | ||
CommonName: awsgo.String(name), | ||
}, | ||
}, | ||
CertificateAuthorityType: awsgo.String("ROOT"), | ||
Tags: []*acmpca.Tag{ | ||
{ | ||
Key: awsgo.String("Name"), | ||
Value: awsgo.String(name), | ||
}, | ||
}, | ||
}) | ||
if err != nil { | ||
assert.Failf(t, "Could not create ACMPCA", errors.WithStackTrace(err).Error()) | ||
} | ||
return ca.CertificateAuthorityArn | ||
} | ||
|
||
func TestListACMPCA(t *testing.T) { | ||
runOrSkip(t) | ||
t.Parallel() | ||
|
||
region, err := getRandomRegion() | ||
if err != nil { | ||
assert.Fail(t, errors.WithStackTrace(err).Error()) | ||
} | ||
session, err := session.NewSession(&awsgo.Config{ | ||
Region: awsgo.String(region)}, | ||
) | ||
|
||
if err != nil { | ||
assert.Fail(t, errors.WithStackTrace(err).Error()) | ||
} | ||
|
||
uniqueTestID := "cloud-nuke-test-" + util.UniqueID() | ||
arn := createTestACMPCA(t, session, uniqueTestID) | ||
// clean up after this test | ||
defer nukeAllACMPCA(session, []*string{arn}) | ||
|
||
newARNs, err := getAllACMPCA(session, region, time.Now().Add(1*time.Hour*-1), config.Config{}) | ||
if err != nil { | ||
assert.Fail(t, "Unable to fetch list of ACMPCA arns") | ||
} | ||
assert.NotContains(t, awsgo.StringValueSlice(newARNs), awsgo.StringValue(arn)) | ||
|
||
allARNs, err := getAllACMPCA(session, region, time.Now().Add(1*time.Hour), config.Config{}) | ||
if err != nil { | ||
assert.Fail(t, "Unable to fetch list of ACMPCA arns") | ||
} | ||
|
||
assert.Contains(t, awsgo.StringValueSlice(allARNs), awsgo.StringValue(arn)) | ||
} | ||
|
||
func TestNukeACMPCA(t *testing.T) { | ||
runOrSkip(t) | ||
t.Parallel() | ||
|
||
region, err := getRandomRegion() | ||
if err != nil { | ||
assert.Fail(t, errors.WithStackTrace(err).Error()) | ||
} | ||
|
||
session, err := session.NewSession(&awsgo.Config{ | ||
Region: awsgo.String(region)}, | ||
) | ||
|
||
if err != nil { | ||
assert.Fail(t, errors.WithStackTrace(err).Error()) | ||
} | ||
|
||
uniqueTestID := "cloud-nuke-test-" + util.UniqueID() | ||
arn := createTestACMPCA(t, session, uniqueTestID) | ||
|
||
if err := nukeAllACMPCA(session, []*string{arn}); err != nil { | ||
assert.Fail(t, errors.WithStackTrace(err).Error()) | ||
} | ||
|
||
arns, err := getAllACMPCA(session, region, time.Now().Add(1*time.Hour), config.Config{}) | ||
if err != nil { | ||
assert.Fail(t, "Unable to fetch list of ACMPCA arns") | ||
} | ||
|
||
assert.NotContains(t, awsgo.StringValueSlice(arns), awsgo.StringValue(arn)) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,36 @@ | ||
package aws | ||
|
||
import ( | ||
awsgo "github.com/aws/aws-sdk-go/aws" | ||
"github.com/aws/aws-sdk-go/aws/session" | ||
"github.com/gruntwork-io/go-commons/errors" | ||
) | ||
|
||
// ACMPA - represents all ACMPA | ||
type ACMPCA struct { | ||
ARNs []string | ||
} | ||
|
||
// ResourceName - the simple name of the aws resource | ||
func (ca ACMPCA) ResourceName() string { | ||
return "acmpca" | ||
} | ||
|
||
// ResourceIdentifiers - The volume ids of the ebs volumes | ||
func (ca ACMPCA) ResourceIdentifiers() []string { | ||
return ca.ARNs | ||
} | ||
|
||
func (ca ACMPCA) MaxBatchSize() int { | ||
// Tentative batch size to ensure AWS doesn't throttle | ||
return 10 | ||
} | ||
|
||
// Nuke - nuke 'em all!!! | ||
func (ca ACMPCA) Nuke(session *session.Session, arns []string) error { | ||
if err := nukeAllACMPCA(session, awsgo.StringSlice(arns)); err != nil { | ||
return errors.WithStackTrace(err) | ||
} | ||
|
||
return nil | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is checking the names regex against the ARN, which is unintuitive. Is there a way to just get the name out?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah. I will think about it. You are correct that this does not feel good.
It should be something unique. And the
name
is not unique.What is your plan about the
regex
behaviour for other resources? Is this "the final form"? Otherwise I would get rid of the regex in total.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would need your guidance on this. Indeed this would be the ARN and there is nothing better than using the ARN for it. Name does not make much sense for this service.
I could introduce new types?
Or I could do:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not the final form, as we plan on supporting tags (something users have requested for other resources). Adding in ARN support or tag support just for ACM PCA makes sense to me, although that would be a lot more work to add considering the potential regressions with other resources.
If it isn't critical for your environment, I might suggest removing the regex filters for now and add that in later as a future feature when we support something that makes more sense, like ARN or tags.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alright. I have removed the
configObj
support (see the latest commit)