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

✨ ✨ Feature/auto approval registration #470

Conversation

ramekris3163
Copy link
Contributor

@ramekris3163 ramekris3163 commented Feb 18, 2025

Summary

  1. Add flags for passing autoApprovedCSRIdentities and autoApprovedARNPatterns as part of init command.
  2. The above flags will be used to validate the CSR or AWS spoke cluster when AutoApproval feature gate is enabled.

Fixes #514

@openshift-ci openshift-ci bot requested review from itdove and yue9944882 February 18, 2025 22:52
@alex0chan alex0chan force-pushed the feature/auto-approval-registration branch from 77b902b to 192ab0a Compare February 26, 2025 22:24
@ramekris3163 ramekris3163 changed the title Feature/auto approval registration ✨ ✨ Feature/auto approval registration Feb 26, 2025
@@ -157,6 +159,32 @@ func (o *Options) validate() error {
}
}

featureGates := o.clusterManagerChartConfig.ClusterManager.RegistrationConfiguration.FeatureGates
Copy link
Member

Choose a reason for hiding this comment

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

I think we have a featuregate flag that you can directly identify rather than check the config. You could check genericclioptionsclusteradm.HubMutableFeatureGate directly

Copy link
Contributor Author

Choose a reason for hiding this comment

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

resolved.

}
}

if managedClusterAutoApprove {
Copy link
Member

Choose a reason for hiding this comment

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

this should be in the Validate func.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is part of the Validate Function.

@@ -81,7 +81,7 @@ func NewCmd(clusteradmFlags *genericclioptionsclusteradm.ClusteradmFlags, stream
_ = clusterManagerSet.SetAnnotation("singleton-name", "singletonSet", []string{})
o.Helm.AddFlags(singletonSet)
cmd.Flags().AddFlagSet(singletonSet)
cmd.Flags().StringArrayVar(&o.registrationAuth, "registration-auth", []string{},
cmd.Flags().StringSliceVar(&o.registrationAuth, "registration-auth", []string{},
Copy link
Member

Choose a reason for hiding this comment

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

registration-drivers since it is plural

Copy link
Contributor Author

Choose a reason for hiding this comment

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

resolved.

@ramekris3163 ramekris3163 force-pushed the feature/auto-approval-registration branch from 1a54036 to 35690fc Compare March 4, 2025 04:27
@jeffw17 jeffw17 force-pushed the feature/auto-approval-registration branch 2 times, most recently from 2e21916 to a51da54 Compare March 4, 2025 21:51
…hitelist for auto approval

Signed-off-by: Jeffrey Wong <jeffreywong0417@gmail.com>
@jeffw17 jeffw17 force-pushed the feature/auto-approval-registration branch from a51da54 to 0727737 Compare March 4, 2025 21:52
if !validRegistrationDriver.Has(driver) {
return fmt.Errorf("only csr and awsirsa are valid drivers")
}
}

featureGates := genericclioptionsclusteradm.ConvertToFeatureGateAPI(
Copy link
Member

Choose a reason for hiding this comment

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

what I meant here is use

if genericclioptionsclusteradm.HubMutableFeatureGate.Enabled("ManagedClusterAutoApproval"):
.....

…oval

Signed-off-by: Jeffrey Wong <jeffreywong0417@gmail.com>
@jeffw17 jeffw17 force-pushed the feature/auto-approval-registration branch from 40e2991 to 438e8cd Compare March 5, 2025 14:17
@qiujian16
Copy link
Member

/approve
/lgtm

Copy link

openshift-ci bot commented Mar 5, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: qiujian16, ramekris3163

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot added the approved label Mar 5, 2025
@openshift-merge-bot openshift-merge-bot bot merged commit b50ec8d into open-cluster-management-io:main Mar 5, 2025
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants