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

Add references to Cognito UserPool #1022

Merged
merged 5 commits into from
Dec 26, 2023

Conversation

mbbush
Copy link
Collaborator

@mbbush mbbush commented Dec 13, 2023

Description of your changes

Adds references to cognito user pool, and makes the example uptestable. I couldn't use the Object.s3 resource because of #978, but the (deprecated in the terraform provider) BucketObject.s3 resource works fine.

I have:

  • Run make reviewable test to ensure this PR is ready for review.

How has this code been tested

make e2e UPTEST_EXAMPLE_LIST=examples/cognitoidp/userpool.yaml

@mbbush mbbush changed the title Cognito references Add references to Cognito UserPool Dec 13, 2023
@mbbush mbbush force-pushed the cognito-references branch from 6e47356 to 534bd8a Compare December 13, 2023 21:49
@mbbush
Copy link
Collaborator Author

mbbush commented Dec 13, 2023

/test-examples="examples/s3/object.yaml"

@mbbush
Copy link
Collaborator Author

mbbush commented Dec 13, 2023

/test-examples="examples/cognitoidp/userpool.yaml"

Comment on lines 57 to 63
r.References["email_configuration.configuration_set"] = config.Reference{
TerraformName: "aws_ses_configuration_set",
}
r.References["email_configuration.source_arn"] = config.Reference{
TerraformName: "aws_ses_email_identity",
Extractor: common.PathARNExtractor,
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

So it turns out there are actually two terraform resources for these objects, these and the corresponding aws_sesv2_ ones. As far as I can tell, they are simply two different interfaces for the same objects, with the same ARNs. I'm conflicted about what to do here, but perhaps these would be better left off?

Copy link
Collaborator

Choose a reason for hiding this comment

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

It seems more logical not to define a reference for these for now.

@mbbush mbbush force-pushed the cognito-references branch from 4112e1b to 11b749f Compare December 22, 2023 00:28
@mbbush
Copy link
Collaborator Author

mbbush commented Dec 22, 2023

/test-examples="examples/cognitoidp/userpool.yaml"

1 similar comment
@turkenf
Copy link
Collaborator

turkenf commented Dec 26, 2023

/test-examples="examples/cognitoidp/userpool.yaml"

Copy link
Collaborator

@turkenf turkenf left a comment

Choose a reason for hiding this comment

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

Thank you @mbbush, LGTM.

@turkenf turkenf merged commit 95376b5 into crossplane-contrib:main Dec 26, 2023
9 checks passed
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.

2 participants