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

Adding support to get details for TargetPrefix #767

Merged
merged 6 commits into from
Feb 10, 2021

Conversation

ina-stoyanova
Copy link
Contributor

This PR adds the functionality needed in this code changes: gruntwork-io/terragrunt#1507

With this PR we can:

  • get the TargetPrefix specified for a state bucket with enabled access logging

Issue linked: gruntwork-io/terragrunt#1437

ina-stoyanova added a commit to gruntwork-io/terragrunt that referenced this pull request Jan 22, 2021
! Note that this needs the terratest PR (gruntwork-io/terratest#767) to be merged in to work & be tested.
}

return aws.StringValue(res.LoggingEnabled.TargetBucket), nil
return aws.StringValue(res.LoggingEnabled.TargetBucket), aws.StringValue(res.LoggingEnabled.TargetPrefix), nil
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To maintain backwards compatibility, should there be a separate method to return just the target prefix? Or possibly the bucket + prefix?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 to introduce a separate method that just retrieves the target prefix.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a really good point! Thanks a lot both 🙂

Copy link
Contributor Author

@ina-stoyanova ina-stoyanova Feb 5, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So in terms of backwards compatibility - did you also mean to keep the naming of the method as it was originally? It was quite a generic name, so there's now 2 functions: GetS3BucketLoggingTargetBucket & GetS3BucketLoggingTargetPrefix

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea, I think preserving the old name makes sense too. It's not an ideal name, but probably not bad enough it merits a backwards incompatible change.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, will change that too - thanks, Jim!

@ina-stoyanova
Copy link
Contributor Author

Took me a while in the midst of support to come back here (I thought it would require a lot more changes, but turns out it was this quick).

Another look would be very appreciated!

Copy link
Member

@brikis98 brikis98 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@ina-stoyanova ina-stoyanova merged commit 6d329f3 into master Feb 10, 2021
@ina-stoyanova
Copy link
Contributor Author

Thanks Jim!

ina-stoyanova added a commit to gruntwork-io/terragrunt that referenced this pull request Mar 11, 2021
* adding target-prefix ro access bucket logging

* Updating test & example

! Note that this needs the terratest PR (gruntwork-io/terratest#767) to be merged in to work & be tested.

* Updating Terratest dependency

* testing for target prefix

* Updating docs

* Renaming folder

* Updating to Debugf

* Adding default value

* WIP - parsing for TFstatelogs

* Updating logic & docs

* Adding a new test for default TargetPrefix in remote backend config
teamfighter added a commit to teamfighter/terragrunt that referenced this pull request Apr 5, 2021
* Fix dead link in multiple aws accounts docs (gruntwork-io#1563)

* Fix dead link in multiple aws accounts docs

The link to AWS docs is now 404.
The corrected link seems to most closely resemble the intended target.
Other options to consider:
https://aws.amazon.com/organizations/getting-started/best-practices/
https://docs.aws.amazon.com/controltower/latest/userguide/aws-multi-account-landing-zone.html

* Link to AWS best practices for multi account docs

* Whitespace removal (gruntwork-io#1573)

* Fix empty outputs (gruntwork-io#1568)

If stack run finished without errors, `summarizePlanAllErrors()`
receives empty buffer and outputs empty line. This change ensures that
only non-empty outputs are getting logged.

Related: gruntwork-io#1541

* doc: contributing: fix broken link to circleci (gruntwork-io#1580)

* Bump AWS SDK to version v1.37.7 to support AWS SSO (gruntwork-io#1537)

* Add TargetPrefix as config input to access bucket logging (gruntwork-io#1507)

* adding target-prefix ro access bucket logging

* Updating test & example

! Note that this needs the terratest PR (gruntwork-io/terratest#767) to be merged in to work & be tested.

* Updating Terratest dependency

* testing for target prefix

* Updating docs

* Renaming folder

* Updating to Debugf

* Adding default value

* WIP - parsing for TFstatelogs

* Updating logic & docs

* Adding a new test for default TargetPrefix in remote backend config

* Introduce validate-inputs, which can be used to check for variable alignment (gruntwork-io#1572)

* Introduce terragrunt-input-info, which can be used to check for variable alignment

* Apply suggestions from code review

Co-authored-by: Zack Proser <[email protected]>

* Tidy go modules

* Renamed input-info to validate-inputs

* Switch missing required vars to errors

* Handle -var and -var-file args

* Update cli/validate_inputs.go

Co-authored-by: Yevgeniy Brikman <[email protected]>

* Make sure to check for dynamically passed in CLI args

* Fix build

* Handle automatically loaded var files

* Remove plan args check

* Clarify difference between getTerraformInputNamesFromVarFiles and getTerraformInputNamesFromCLIArgs

* Address PR nit to move example in docs

Co-authored-by: Zack Proser <[email protected]>
Co-authored-by: Yevgeniy Brikman <[email protected]>

* Use go1.16 to build arm64 binaries (gruntwork-io#1585)

* Bump creack/pty to 1.1.11 (gruntwork-io#1582)

Co-authored-by: Andy Bohne <[email protected]>

* Add ability to specify working directory of hooks (gruntwork-io#1588)

* Add ability to specify working directory of hooks

* Fix build

* Support dynamodb_endpoint attribute of S3 backend (gruntwork-io#1586)

* Clarify non-interactive will not include external dependencies (gruntwork-io#1593)

* add getTerragruntSource helper function (gruntwork-io#1575)

* add getTerragruntSource helper function

* update docs

* update docs and tests for get_terragrunt_source_cli_flag() function

* add use cases for get_terragrunt_source_cli_flag

* Recursively extract forcedgetters until there are none (gruntwork-io#1594)

* Remove all usage of get-plugins=false which is removed in 0.15.0 (gruntwork-io#1618)

* Fix validate-inputs to support null defaults (gruntwork-io#1613)

* Clarify context of find_in_parent_folders (gruntwork-io#1623)

Co-authored-by: Paul <[email protected]>
Co-authored-by: Yoriyasu Yano <[email protected]>
Co-authored-by: amnk <[email protected]>
Co-authored-by: Marco Molteni <[email protected]>
Co-authored-by: David Wooldridge <[email protected]>
Co-authored-by: Ina Stoyanova <[email protected]>
Co-authored-by: Zack Proser <[email protected]>
Co-authored-by: Yevgeniy Brikman <[email protected]>
Co-authored-by: Andy Bohne <[email protected]>
Co-authored-by: Andy Bohne <[email protected]>
Co-authored-by: Alexey Remizov <[email protected]>
Co-authored-by: Syed Hussain <[email protected]>
Co-authored-by: David Alger <[email protected]>
@ina-stoyanova ina-stoyanova deleted the add-target-prefix-s3-state-bucket branch April 23, 2021 16:04
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.

3 participants