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 aws_msk_scram_secret_association to v1beta1 #836

Merged
merged 6 commits into from
Sep 27, 2023

Conversation

mbbush
Copy link
Collaborator

@mbbush mbbush commented Aug 17, 2023

Description of your changes

Added MSK scram secret association, generated from the terraform provider.

Note: the terraform provider has some open bugs related to this resource, but most significantly it seems to assume that there is at most one of these per MSK cluster. Creating more than one aws_msk_scram_secret_association using provider-terraform results in the two resources each attempting to make their secret the only one associated with the cluster. I'd love it if there was a way for me to make this more visible to the user. Can I just customize the description field(s) in config/kafka/config.go?

Fixes #43

I have:

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

How has this code been tested

NAME                         READY   SYNCED   EXTERNAL-NAME           AGE
vpc.ec2.aws.upbound.io/vpc   True    True     vpc-007c5ee139c94e7c1   16h

NAME                                   READY   SYNCED   EXTERNAL-NAME              AGE
subnet.ec2.aws.upbound.io/subnet-az1   True    True     subnet-080f8fe5b4e83d7d1   16h
subnet.ec2.aws.upbound.io/subnet-az2   True    True     subnet-086f2896978d377b2   16h

NAME                                  READY   SYNCED   EXTERNAL-NAME          AGE
securitygroup.ec2.aws.upbound.io/sg   True    True     sg-07ac2fe4a3d3e7a43   16h

NAME                                                  READY   SYNCED   EXTERNAL-NAME                                                                                 AGE
scramsecretassociation.kafka.aws.upbound.io/example   True    True     arn:aws:kafka:us-east-2:REDACTED:cluster/example/459fcc82-84f7-47ea-b3d5-b04626872fe0-2   16h

NAME                                   READY   SYNCED   EXTERNAL-NAME                                                                                 AGE
cluster.kafka.aws.upbound.io/example   True    True     arn:aws:kafka:us-east-2:REDACTED:cluster/example/459fcc82-84f7-47ea-b3d5-b04626872fe0-2   16h

NAME                                         READY   SYNCED   EXTERNAL-NAME                                                                                       AGE
configuration.kafka.aws.upbound.io/example   True    True     arn:aws:kafka:us-east-2:REDACTED:configuration/example/2cb17c03-8a01-4595-8239-13c1f74bbaea-2   15h

NAME                             READY   SYNCED   EXTERNAL-NAME                          AGE
key.kms.aws.upbound.io/example   True    True     a6dfae8c-6e9b-4137-bd0c-f6181a2a780a   16h

NAME                                            READY   SYNCED   EXTERNAL-NAME                                                                    AGE
secret.secretsmanager.aws.upbound.io/example1   True    True     arn:aws:secretsmanager:us-east-2:REDACTED:secret:AmazonMSK_example1-NDtz9u   16h
secret.secretsmanager.aws.upbound.io/example2   True    True     arn:aws:secretsmanager:us-east-2:REDACTED:secret:AmazonMSK_example2-DeXdak   16h

NAME                                                   READY   SYNCED   EXTERNAL-NAME                                                                                                         AGE
secretversion.secretsmanager.aws.upbound.io/example1   True    True     arn:aws:secretsmanager:us-east-2:REDACTED:secret:AmazonMSK_example1-NDtz9u|090287EE-3B4A-43E4-B8BD-7B135B1736EA   16h
secretversion.secretsmanager.aws.upbound.io/example2   True    True     arn:aws:secretsmanager:us-east-2:REDACTED:secret:AmazonMSK_example2-DeXdak|18D028A4-CAF3-46D5-BBCE-EA8F1BC4B2C1   16h

With these managed resources in this state (applied from examples/scramsecretassociation.yaml,) I can see both secrets listed as associated with the MSK cluster in the aws console.

make e2e UPTEST_EXAMPLE_LIST='examples/kafka/scramsecretassociation.yaml'
After a lot of babysitting, some manual intervention around the kafka cluster's configuration and a whole lot of waiting, the tests passed:

--- PASS: kuttl (4267.27s)
    --- PASS: kuttl/harness (0.00s)
        --- PASS: kuttl/harness/case (4243.12s)
PASS
17:54:28 [ OK ] running automated tests

Uptest: https://github.com/upbound/provider-aws/actions/runs/6297489952

@Upbound-CLA
Copy link

Upbound-CLA commented Aug 17, 2023

CLA assistant check
All committers have signed the CLA.

@mbbush
Copy link
Collaborator Author

mbbush commented Aug 17, 2023

/test-examples="examples/kafka/scramsecretassociation.yaml"

EDIT: I don't know what this is supposed to trigger or where I can see the results.

@jeanduplessis
Copy link
Collaborator

@mbbush the test command can only be triggered by Upbound org members.

@jeanduplessis
Copy link
Collaborator

/test-examples="examples/kafka/scramsecretassociation.yaml"

@mbbush
Copy link
Collaborator Author

mbbush commented Sep 1, 2023

@jeanduplessis Unfortunately I didn't get a chance to look at the logs for the test failure, and now they're expired. Could you run it again? Or even better, is there an easy way I can run the same thing locally?

@jeanduplessis
Copy link
Collaborator

/test-examples="examples/kafka/scramsecretassociation.yaml"

@mbbush mbbush force-pushed the msk-sasl-scram-association branch 2 times, most recently from 5c7d500 to 67daba6 Compare September 12, 2023 16:42
@mbbush mbbush marked this pull request as ready for review September 12, 2023 16:42
@mbbush mbbush force-pushed the msk-sasl-scram-association branch from 67daba6 to e108c80 Compare September 13, 2023 00:48
@mbbush
Copy link
Collaborator Author

mbbush commented Sep 21, 2023

If we can get #877 merged I can update the example to pass without manual intervention.

@mbbush mbbush changed the title Msk sasl scram association Adding aws_msk_scram_secret_association to v1beta1 Sep 21, 2023
@mbbush mbbush force-pushed the msk-sasl-scram-association branch from e108c80 to a813532 Compare September 24, 2023 21:29
@mbbush
Copy link
Collaborator Author

mbbush commented Sep 25, 2023

@turkenf this should be ready to merge.

@turkenf
Copy link
Collaborator

turkenf commented Sep 25, 2023

/test-examples="examples/kafka/scramsecretassociation.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 for your effort in this PR @mbbush, LGTM.

Before merging, can you please make sure we have removed from nottested?

@turkenf
Copy link
Collaborator

turkenf commented Sep 26, 2023

@mbbush we have the plan to cut the release on Thursday this week. If you want to include this PR in the release, could you please remove the lines about the resource from provideraws/config/externalnamenottested.go?

@mbbush
Copy link
Collaborator Author

mbbush commented Sep 26, 2023

Done, thanks!

@mbbush
Copy link
Collaborator Author

mbbush commented Sep 27, 2023

I also added a more verbose description to the secretArnList to clarify that you can really only have one ScramSecretAssociation per MSK cluster.

@turkenf turkenf merged commit 90e6de0 into crossplane-contrib:main Sep 27, 2023
8 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.

Moving kafka (1) resources to v1beta1 version
4 participants