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

✨Updated clustermanager API spec and generated files for spec change. #355

Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,18 @@ spec:
- feature
type: object
type: array
registrationDriverHub:
description: This provides details to initialize Hub cluster
properties:
authType:
default: csr
description: Type of the authentication used by hub to initialize
the Hub cluster. Possible values are csr and awsirsa.
enum:
- csr
- awsirsa
type: string
type: object
type: object
registrationImagePullSpec:
default: quay.io/open-cluster-management/registration
Expand Down
13 changes: 13 additions & 0 deletions operator/v1/types_clustermanager.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,19 @@ type RegistrationHubConfiguration struct {
// he can set featuregate/Foo=false before upgrading. Let's say the cluster-admin wants featuregate/Foo=false.
// +optional
FeatureGates []FeatureGate `json:"featureGates,omitempty"`

// This provides details to initialize Hub cluster
Copy link
Member

Choose a reason for hiding this comment

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

the comment needs to be updated, it is not clear what this drivers meant to do.

Copy link
Contributor

Choose a reason for hiding this comment

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

Modified the comment to give more clarity

// +optional
RegistrationDriverHub RegistrationDriverHub `json:"registrationDriverHub,omitempty"`
Copy link
Member

Choose a reason for hiding this comment

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

we can call it authDriver or registrationDriver. Also I think this should be a list, we should be able to support multiple drivers at the same time on hub.

Copy link
Contributor

Choose a reason for hiding this comment

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

Updated with the suggestions

Copy link
Member

Choose a reason for hiding this comment

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

could you make this as

registrationDrivers []RegistrationDriver

Copy link
Contributor

Choose a reason for hiding this comment

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

We already have another struct with name RegistrationDriver in types_klusterlet.go. That is the reason it was initially named it as RegistrationDriverHub. So can we go with the second option that you had suggested which is authDriver?

Copy link
Member

Choose a reason for hiding this comment

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

it is ok that the struct name is RegistrationDriverHub, but let's make the field name to be registrationDrivers and make it as a slice.

Copy link
Contributor

Choose a reason for hiding this comment

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

Renamed to registrationDrivers and it was already changed to a slice in previous commit

}

type RegistrationDriverHub struct {

// Type of the authentication used by hub to initialize the Hub cluster. Possible values are csr and awsirsa.
// +required
// +kubebuilder:default:=csr
// +kubebuilder:validation:Enum=csr;awsirsa
AuthType string `json:"authType,omitempty"`
Copy link
Member

Choose a reason for hiding this comment

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

one minor comment, we should add

// +listType=map
// +listMapKey=authType
here, so we will not set duplicate type

}

type WorkConfiguration struct {
Expand Down
17 changes: 17 additions & 0 deletions operator/v1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

13 changes: 11 additions & 2 deletions operator/v1/zz_generated.swagger_doc_generated.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading