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.

Sign up for GitHub

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

Jump to bottom

Add MQ User resource #1311

Merged
merged 5 commits into from
May 23, 2024
Merged

Add MQ User resource #1311

merged 5 commits into from
May 23, 2024

Conversation

mergenci
Copy link
Collaborator

@mergenci mergenci commented May 14, 2024 •
edited
Loading

Description of your changes

Add MQ User resource by consuming upbound/terraform-provider-aws#236.

I have:

  • Read and followed Crossplane's contribution process.
  • Run make reviewable to ensure this PR is ready for review.
  • Added backport release-x.y labels to auto-backport this PR if necessary.

How has this code been tested

Manually tested by applying the example manifest.

ulucinar
ulucinar reviewed May 14, 2024
View reviewed changes
Copy link
Collaborator

@ulucinar ulucinar 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 @mergenci for working on the new Terraform & Crossplane resource for managing ActiveMQ users.

config/mq/config.go Outdated Show resolved Hide resolved
@mergenci mergenci force-pushed the add-mq-user-resource branch 2 times, most recently from 5d2373f to 085a38d Compare May 23, 2024 13:46
ulucinar
ulucinar approved these changes May 23, 2024
View reviewed changes
Copy link
Collaborator

@ulucinar ulucinar left a comment

Choose a reason for hiding this comment

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

Thanks @mergenci, lgtm. Left some comments for you to consider. I think we can merge this PR although we will need crossplane/upjet#406 and crossplane/upjet#407 to prevent a race condition between the User.mq and Broker.mq controllers.

Makefile Show resolved Hide resolved
config/externalname.go
Comment on lines +2754 to +2756
if externalName == "" {
return "invalidnonemptystring", nil
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

We are planning to take a second look into this to see whether we can avoid it.

examples/mq/v1alpha1/user.yaml Show resolved Hide resolved
examples/mq/v1alpha1/user.yaml Show resolved Hide resolved
@mergenci mergenci force-pushed the add-mq-user-resource branch from 085a38d to f5ec9b4 Compare May 23, 2024 14:36
@ulucinar
Copy link
Collaborator

Let's wait for #1315 before we merge this...

@mergenci mergenci marked this pull request as ready for review May 23, 2024 14:36
mergenci and others added 3 commits May 23, 2024 18:00
@mergenci
Add MQ User resource.
64f0a25 
Signed-off-by: Cem Mergenci <[email protected]>
@ulucinar @mergenci
Fix the Broker.mq reference in User.mq
23df338 
- Add an example manifest for User.mq

Signed-off-by: Alper Rifat Ulucinar <[email protected]>
@ulucinar @mergenci
Use a smaller host machine in the User.mq example manifest
5c926cb 
Signed-off-by: Alper Rifat Ulucinar <[email protected]>
@mergenci mergenci force-pushed the add-mq-user-resource branch from f202934 to 5932530 Compare May 23, 2024 15:07
@mergenci mergenci force-pushed the add-mq-user-resource branch from 5932530 to 847d1a5 Compare May 23, 2024 15:30
@ulucinar
Copy link
Collaborator

To prevent the race between the new User.mq and Broker.mq controllers, we will need to consume crossplane/upjet#406 & crossplane/upjet#407 and do a late-initialization configuration.

@ulucinar
Copy link
Collaborator

Hi @mergenci,
A local run of make lint on the head of this PR's feature branch has succeeded. No need to wait for the linter job if you'd like to continue merging...

@mergenci
Copy link
Collaborator Author

Thanks @ulucinar 🙏 This PR has been a wild ride in many ways 🙂

@mergenci mergenci merged commit 739e4ea into crossplane-contrib:main May 23, 2024
10 of 11 checks passed
@mergenci mergenci deleted the add-mq-user-resource branch May 23, 2024 18:36
This was referenced May 24, 2024
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ulucinar ulucinar ulucinar approved these changes

@sergenyalcin sergenyalcin Awaiting requested review from sergenyalcin sergenyalcin is a code owner

@turkenf turkenf Awaiting requested review from turkenf turkenf is a code owner

@mbbush mbbush Awaiting requested review from mbbush mbbush is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@mergenci @ulucinar