✨ Update clustermanager API spec for auto approval identities #357
Conversation
There was a problem hiding this comment.
Type: "FieldValueRequired",
Message: "Required value: this property is in x-kubernetes-list-map-keys, so it must have a default or be a required property",
Field: "spec.validation.openAPIV3Schema.properties[spec].properties[registrationConfiguration].properties[autoApprovalIdentities].items.properties[driver].default",
RegistrationDrivers didn't have this issue so maybe use that as reference.
jeffw17
force-pushed
the
feature/auto-approval-registration
branch
2 times, most recently
from
92ff577 to
9492e04
Compare
Signed-off-by: “Jeffrey <jeffreywong0417@gmail.com>
jeffw17
force-pushed
the
feature/auto-approval-registration
branch
from
9492e04 to
ec28f1b
Compare
|
/assign @skeeey @xuezhaojun |
|
We may need to mark the |
operator/v1/types_clustermanager.go
Outdated
| // +kubebuilder:validation:Enum=csr;awsirsa | ||
| Driver string `json:"driver,omitempty"` | ||
|
|
||
| // Identities represent a list of users in which we will allow to join with hub cluster |
There was a problem hiding this comment.
may need more comments here
- for csr driver, the identities represent a list of users ...
- for awsirsa driver, the identities represent a list of arns ...
|
rethinking of this...how about we add approvedIdentities in the registrationDrivers field for each driver? It will make the API easier to use. |
Signed-off-by: Amrutha <amrutha.hari12@gmail.com>
operator/v1/types_clustermanager.go
Outdated
| @@ -115,6 +115,13 @@ type RegistrationHubConfiguration struct { | |||
| // +listType=map | |||
| // +listMapKey=authType | |||
| RegistrationDrivers []RegistrationDriverHub `json:"registrationDrivers,omitempty"` | |||
|
|
|||
| // AutoApprovalIdentities represent the list of approved identities which is used to whitelist certain identities to join with the hub cluster | |||
| // An ApprovedIdentities contains details of the driver type (csr, awsirsa) and a list of identities to whitelist. | |||
There was a problem hiding this comment.
also need to explain this will override the approvedUsers field if it is set.
operator/v1/types_clustermanager.go
Outdated
| Driver string `json:"driver,omitempty"` | ||
|
|
||
| // Identities represent a list of users in which we will allow to join with hub cluster | ||
| // +required |
There was a problem hiding this comment.
also need to set the minimumItem to 1.
operator/v1/types_clustermanager.go
Outdated
| // For csr authentication type, Identities represent a list of approved users | ||
| // For awsirsa authentication type, Identities represent a list of approved arn patterns | ||
| // +optional | ||
| Identities []string `json:"identities,omitempty"` |
There was a problem hiding this comment.
maybe approveIdentities or autoApprovedIdentities?
Signed-off-by: “Jeffrey <jeffreywong0417@gmail.com>
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jeffw17, qiujian16 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-merge-bot
bot
merged commit 4281b76
into
open-cluster-management-io:main
Summary
We want to add a new field in the RegistrationHubConfiguration for clustermanager to store AutoApprovalIdentities for both CSR and AWSIRSA auth type.
Related issue(s)
Fixes # #514 and #822