Refactor: move common functions out of csr controller, into constants or helpers.
#428
Conversation
…s` or `helpers`. Signed-off-by: xuezhaojun <[email protected]>
There was a problem hiding this comment.
Copilot reviewed 5 out of 7 changed files in this pull request and generated no suggestions.
Files not reviewed (2)
- pkg/controller/csr/csr_controller.go: Evaluated as low risk
- pkg/controller/csr/csr_controller_test.go: Evaluated as low risk
Comments skipped due to low confidence (1)
pkg/controller/controller.go:49
- Ensure that the new behavior introduced by
extraApprovalConditionsincsr.Addis covered by tests.
[]func(ctx context.Context, csr *certificatesv1.CertificateSigningRequest) (bool, error){})
|
/assign @zhujian7 |
| ) | ||
|
|
||
| var log = logf.Log.WithName("controller_csr") | ||
|
|
||
| func getClusterName(csr *certificatesv1.CertificateSigningRequest) (clusterName string) { |
There was a problem hiding this comment.
This is the first change, move to helpers pkg
| ) | ||
|
|
||
| // Add creates a new CSR Controller and adds it to the Manager. The Manager will set fields on the Controller | ||
| // and Start it when the Manager is Started. | ||
| func Add(ctx context.Context, | ||
| mgr manager.Manager, | ||
| clientHolder *helpers.ClientHolder) error { | ||
| clientHolder *helpers.ClientHolder, | ||
| extraApprovalConditions []func(ctx context.Context, csr *certificatesv1.CertificateSigningRequest) (bool, error)) error { |
There was a problem hiding this comment.
Here is the second change, now the Add function accept extraApprovalConditions.
|
/lgtm |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: xuezhaojun, zhujian7 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 |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: xuezhaojun, zhujian7 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 |
|
There are mainly 2 changes in this PR:
First, the
GetClusterNameis moved fromcsr-controllertohelperspkg, along with some const values move toconstantpkg.Second, the
Addfunction added aextraApprovalConditionsarray, to allow thecsr controllerto accept and use customized approval conditions.What I want to achieve, is in the future, we can have more approvalConditions defined and implementated in their our pkg(for example, flightctl approve stays in the
flightctlpkg with other flight logic code), when we add new approval condition, thecsr controllerdon't need to change much.