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

Support cross-region replication #128

Closed
wants to merge 7 commits into from
Closed

Conversation

Nuru
Copy link
Contributor

@Nuru Nuru commented Apr 16, 2023

what

why

references

Copy link

@bridgecrew bridgecrew bot left a comment

Choose a reason for hiding this comment

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

Bridgecrew has found errors in this PR ⬇️

dynamodb.tf Show resolved Hide resolved
modules/s3-bucket/main.tf Show resolved Hide resolved
modules/s3-bucket/main.tf Show resolved Hide resolved
modules/s3-bucket/main.tf Show resolved Hide resolved
dynamodb.tf Show resolved Hide resolved
modules/s3-bucket/main.tf Show resolved Hide resolved
modules/s3-bucket/main.tf Show resolved Hide resolved
modules/s3-bucket/main.tf Show resolved Hide resolved
modules/s3-bucket/main.tf Show resolved Hide resolved
modules/s3-bucket/main.tf Show resolved Hide resolved
@Nuru
Copy link
Contributor Author

Nuru commented Apr 16, 2023

/test all

@@ -0,0 +1,3 @@
variable "region" {
type = string
Copy link
Member

Choose a reason for hiding this comment

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

add description

version = ">= 1.3"
source = "hashicorp/aws"
version = ">= 4.0"
configuration_aliases = [aws.blue, aws.green]
Copy link
Member

@aknysh aknysh Apr 16, 2023

Choose a reason for hiding this comment

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

this new syntax is nice, but it generates the error

Error: missing provider provider["registry.terraform.io/hashicorp/aws"].green

and it looks like the issue is not solved yet (adding provider block solves one issue but produces another one)

hashicorp/terraform#28490

Copy link
Contributor Author

@Nuru Nuru Apr 16, 2023

Choose a reason for hiding this comment

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

I am just going to disable the validate test on the module, since that issue clearly says that validate is not expected to work on child modules.

@Nuru Nuru requested a review from aknysh April 17, 2023 00:41
@Nuru
Copy link
Contributor Author

Nuru commented Apr 17, 2023

/test all

Comment on lines +15 to +16
There is therefore no longer a block against unencrypted uploads, since they
will be immediately encrypted anyway.
Copy link
Member

Choose a reason for hiding this comment

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

Doesn't this mean we're no longer enforcing the following?:

 "Condition": {
        "Bool": {
          "aws:SecureTransport": "false"
        }
      },

If so, some of the compliance checks are going to fail. This means that the object isn't required to be encrypted in transit.

@Gowiem
Copy link
Member

Gowiem commented Mar 6, 2024

Hey @Nuru @mcalhoun @aknysh is this worth keeping around? Totally fine if it's something ya'll are planning on circling back on, but figured I should raise the questions considering it has drifted so much and is very stale at this point.

@hans-d hans-d added the stale This PR has gone stale label Mar 7, 2024
Copy link

mergify bot commented Mar 7, 2024

This pull request is now in conflict. Could you fix it @Nuru? 🙏

@hans-d
Copy link
Contributor

hans-d commented Mar 7, 2024

Closing due to staleness, feel free to reopen and update into a reviewable state.

@hans-d hans-d closed this Mar 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale This PR has gone stale
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants