Skip to content
This repository has been archived by the owner on Nov 16, 2022. It is now read-only.

adds attributes field to KeycloakUser CRD #171

Merged
merged 1 commit into from
May 5, 2020
Merged

adds attributes field to KeycloakUser CRD #171

merged 1 commit into from
May 5, 2020

Conversation

jjaferson
Copy link
Contributor

JIRA ID

Adds attributes field to KeycloakUser CRD.

https://issues.redhat.com/browse/INTLY-6491

Additional Information

The attributes field is missing in KeycloakUser CRD and we want to be able to use this field to flag when users are created in other products in the RHMI operator

https://www.keycloak.org/docs-api/9.0/rest-api/index.html#_userrepresentation

@coveralls
Copy link

coveralls commented Apr 17, 2020 •
edited

Coverage Status

Coverage decreased (-0.08%) to 40.937% when pulling 20307c6 on jjaferson:keycloakuser-cr into 065b598 on keycloak:master.

@matskiv
Copy link
Contributor

matskiv commented Apr 17, 2020

ping @slaskawi @mhajas

@slaskawi slaskawi requested a review from mhajas April 20, 2020 07:47
@slaskawi
Copy link
Contributor

@mhajas May I ask you to review this?

@slaskawi slaskawi added this to the 9.0.3 milestone Apr 20, 2020
mhajas
mhajas suggested changes Apr 21, 2020
View reviewed changes
Copy link
Contributor

@mhajas mhajas 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 the PR @jjaferson! There seems to be a bug though, see comments in code.

pkg/apis/keycloak/v1alpha1/keycloakuser_types.go Outdated Show resolved Hide resolved
deploy/crds/keycloak.org_keycloakrealms_crd.yaml Outdated Show resolved Hide resolved
deploy/crds/keycloak.org_keycloakusers_crd.yaml Outdated Show resolved Hide resolved
@slaskawi
Copy link
Contributor

@jjaferson Could you please address @mhajas comments?

@slaskawi slaskawi removed this from the 9.0.3 milestone Apr 24, 2020
@jjaferson
Copy link
Contributor Author

@jjaferson Could you please address @mhajas comments?

thanks for the heads up @slaskawi I have just done the changes required by @mhajas

Thanks guys

@slaskawi
Copy link
Contributor

slaskawi commented May 4, 2020

The code LGTM. One more change request - would you mind to remove go.sum changes, @jjaferson ?

@slaskawi slaskawi added this to the 10.0.0 milestone May 4, 2020
@jjaferson jjaferson force-pushed the keycloakuser-cr branch 3 times, most recently from 3417adc to 7254769 Compare May 5, 2020 08:45
@jjaferson
Copy link
Contributor Author

The code LGTM. One more change request - would you mind to remove go.sum changes, @jjaferson ?

done

slaskawi
slaskawi previously approved these changes May 5, 2020
View reviewed changes
Copy link
Contributor

@slaskawi slaskawi left a comment

Choose a reason for hiding this comment

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

LGTM. @mhajas May I ask you for a review?

mhajas
mhajas suggested changes May 5, 2020
View reviewed changes
Copy link
Contributor

@mhajas mhajas left a comment

Choose a reason for hiding this comment

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

Hey @jjaferson, I still see some change in go.sum. Is it necessary?

@jjaferson
Copy link
Contributor Author

Hey @jjaferson, I still see some change in go.sum. Is it necessary?

No, not at all. it was only a space that I removed already.

mhajas
mhajas approved these changes May 5, 2020
View reviewed changes
Copy link
Contributor

@mhajas mhajas left a comment

Choose a reason for hiding this comment

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

Ok, thanks! Now it looks good to me.

@slaskawi slaskawi merged commit 64bad71 into keycloak:master May 5, 2020
@davidffrench davidffrench mentioned this pull request Jun 2, 2020
Merged
5 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@slaskawi slaskawi slaskawi approved these changes

@mhajas mhajas mhajas approved these changes

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

Successfully merging this pull request may close these issues.

None yet
5 participants
@jjaferson @coveralls @matskiv @slaskawi @mhajas