Conversation
CLI Command: go run ./cmd/scaffold-controller -interactive=false \ -kind User \ -gophercloud-client NewIdentityV3 \ -gophercloud-module github.com/gophercloud/gophercloud/v2/openstack/identity/v3/users \ -import-dependency Domain \ -optional-create-dependency Domain Signed-off-by: Daniel Lawton <dlawton@redhat.com>
dlaw4608
force-pushed
the
keystone_user
branch
2 times, most recently
from
23295c5 to
dcfe645
Compare
- E2E tests included - API configured - Controller Note: small fix made to group-import test included Signed-off-by: Daniel Lawton <dlawton@redhat.com>
dlaw4608
force-pushed
the
keystone_user
branch
from
dcfe645 to
0f90e6a
Compare
winiciusallan
left a comment
winiciusallan
left a comment
There was a problem hiding this comment.
Hi, @dlaw4608, that was a great job!
Left a few comments about using a Project object instead of the ID directly, LMK what you think.
| // +kubebuilder:validation:MinLength:=1 | ||
| // +kubebuilder:validation:MaxLength:=255 | ||
| // +optional | ||
| DefaultProjectID *string `json:"defaultProjectID,omitempty"` |
There was a problem hiding this comment.
Isn't it better to use a ProjectRef here so we can better control the resource?
| DefaultProjectID *string `json:"defaultProjectID,omitempty"` | |
| DefaultProjectID *ProjectRef `json:"defaultProjectID,omitempty"` |
There was a problem hiding this comment.
It should indeed be a reference, but would rather be in the form:
| DefaultProjectID *string `json:"defaultProjectID,omitempty"` | |
| DefaultProjectRef *KubernetesNameRef `json:"defaultProjectRef,omitempty"` |
See https://k-orc.cloud/development/architecture/#api-design-philosophy.
I'd like to add a lint rule to flag these.
Also, since this is an optional dependency for your controller, you should have run the scaffolding tool with -optional-create-dependency Project.
| } | ||
| } | ||
|
|
||
| func handleDefaultProjectIDUpdate(updateOpts *users.UpdateOpts, resource *resourceSpecT, osResource *osResourceT) { |
There was a problem hiding this comment.
Possibly change this to use a reference instead of an ID as suggested above.
|
|
||
|
|
||
|
|
||
|
|
||
|
|
There was a problem hiding this comment.
Blank lines at the end. I would remove them to make it cleaner.
| resource: | ||
| name: user-create-minimal |
There was a problem hiding this comment.
I would also add enabled: true here.
| // +kubebuilder:validation:MinLength:=1 | ||
| // +kubebuilder:validation:MaxLength:=255 | ||
| // +optional | ||
| DefaultProjectID *string `json:"defaultProjectID,omitempty"` |
There was a problem hiding this comment.
It should indeed be a reference, but would rather be in the form:
| DefaultProjectID *string `json:"defaultProjectID,omitempty"` | |
| DefaultProjectRef *KubernetesNameRef `json:"defaultProjectRef,omitempty"` |
See https://k-orc.cloud/development/architecture/#api-design-philosophy.
I'd like to add a lint rule to flag these.
Also, since this is an optional dependency for your controller, you should have run the scaffolding tool with -optional-create-dependency Project.
| // +kubebuilder:validation:MinLength:=1 | ||
| // +kubebuilder:validation:MaxLength:=255 | ||
| // +optional | ||
| Password *string `json:"password,omitempty"` |
There was a problem hiding this comment.
This must be stored in a secret. See how we do it for user data in the server controller.
3 participants
Fixes #583
Add Support for the Identity service's User resource in ORC