Skip to content

Commit 89d6670

Browse files
inteoninteon
committed
remove GetIssuerTypeIdentifier from issuer API, moving it to the controller initialisation
Signed-off-by: Tim Ramlot <42113979+inteon@users.noreply.github.com>
1 parent c77d8be commit 89d6670

15 files changed

+221
-156
lines changed

‎api/v1alpha1/issuer_interface.go

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -27,15 +27,6 @@ import (
2727
type Issuer interface {
2828
runtime.Object
2929
metav1.Object
30-
GetStatus() *IssuerStatus
3130

32-
// GetIssuerTypeIdentifier returns a string that uniquely identifies the
33-
// issuer type. This should be a constant across all instances of this
34-
// issuer type. This string is used as a prefix when determining the
35-
// issuer type for a Kubernetes CertificateSigningRequest resource based
36-
// on the issuerName field. The value should be formatted as follows:
37-
// "<issuer resource (plural)>.<issuer group>". For example, the value
38-
// "simpleclusterissuers.issuer.cert-manager.io" will match all CSRs
39-
// with an issuerName set to eg. "simpleclusterissuers.issuer.cert-manager.io/issuer1".
40-
GetIssuerTypeIdentifier() string
31+
GetStatus() *IssuerStatus
4132
}

‎controllers/certificaterequest_controller_integration_test.go

Lines changed: 21 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ import (
3232
"github.com/stretchr/testify/require"
3333
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
3434
"k8s.io/apimachinery/pkg/runtime"
35+
"k8s.io/apimachinery/pkg/runtime/schema"
3536
"k8s.io/apimachinery/pkg/types"
3637
"k8s.io/apimachinery/pkg/watch"
3738
"k8s.io/client-go/tools/record"
@@ -84,12 +85,16 @@ func TestCertificateRequestControllerIntegrationIssuerInitiallyNotFoundAndNotRea
8485
func(mgr ctrl.Manager) controllerInterface {
8586
return &CertificateRequestReconciler{
8687
RequestController: RequestController{
87-
IssuerTypes: []v1alpha1.Issuer{&api.TestIssuer{}},
88-
ClusterIssuerTypes: []v1alpha1.Issuer{&api.TestClusterIssuer{}},
89-
FieldOwner: fieldOwner,
90-
MaxRetryDuration: time.Minute,
91-
EventSource: kubeutil.NewEventStore(),
92-
Client: mgr.GetClient(),
88+
IssuerTypes: map[schema.GroupResource]v1alpha1.Issuer{
89+
api.TestIssuerGroupVersionResource.GroupResource(): &api.TestIssuer{},
90+
},
91+
ClusterIssuerTypes: map[schema.GroupResource]v1alpha1.Issuer{
92+
api.TestClusterIssuerGroupVersionResource.GroupResource(): &api.TestClusterIssuer{},
93+
},
94+
FieldOwner: fieldOwner,
95+
MaxRetryDuration: time.Minute,
96+
EventSource: kubeutil.NewEventStore(),
97+
Client: mgr.GetClient(),
9398
Sign: func(_ context.Context, cr signer.CertificateRequestObject, _ v1alpha1.Issuer) (signer.PEMBundle, error) {
9499
atomic.AddUint64(&counters[extractIdFromNamespace(t, cr.GetNamespace())], 1)
95100
return signer.PEMBundle{
@@ -223,12 +228,16 @@ func TestCertificateRequestControllerIntegrationSetCondition(t *testing.T) {
223228
func(mgr ctrl.Manager) controllerInterface {
224229
return &CertificateRequestReconciler{
225230
RequestController: RequestController{
226-
IssuerTypes: []v1alpha1.Issuer{&api.TestIssuer{}},
227-
ClusterIssuerTypes: []v1alpha1.Issuer{&api.TestClusterIssuer{}},
228-
FieldOwner: fieldOwner,
229-
MaxRetryDuration: time.Minute,
230-
EventSource: kubeutil.NewEventStore(),
231-
Client: mgr.GetClient(),
231+
IssuerTypes: map[schema.GroupResource]v1alpha1.Issuer{
232+
api.TestIssuerGroupVersionResource.GroupResource(): &api.TestIssuer{},
233+
},
234+
ClusterIssuerTypes: map[schema.GroupResource]v1alpha1.Issuer{
235+
api.TestClusterIssuerGroupVersionResource.GroupResource(): &api.TestClusterIssuer{},
236+
},
237+
FieldOwner: fieldOwner,
238+
MaxRetryDuration: time.Minute,
239+
EventSource: kubeutil.NewEventStore(),
240+
Client: mgr.GetClient(),
232241
Sign: func(ctx context.Context, cr signer.CertificateRequestObject, _ v1alpha1.Issuer) (signer.PEMBundle, error) {
233242
atomic.AddUint64(&counter, 1)
234243
select {

‎controllers/certificaterequest_controller_test.go

Lines changed: 54 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ import (
3131
"github.com/stretchr/testify/require"
3232
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
3333
"k8s.io/apimachinery/pkg/runtime"
34+
"k8s.io/apimachinery/pkg/runtime/schema"
3435
"k8s.io/apimachinery/pkg/types"
3536
"k8s.io/client-go/tools/record"
3637
clocktesting "k8s.io/utils/clock/testing"
@@ -942,15 +943,19 @@ func TestCertificateRequestReconcilerReconcile(t *testing.T) {
942943

943944
controller := (&CertificateRequestReconciler{
944945
RequestController: RequestController{
945-
IssuerTypes: []v1alpha1.Issuer{&api.TestIssuer{}},
946-
ClusterIssuerTypes: []v1alpha1.Issuer{&api.TestClusterIssuer{}},
947-
FieldOwner: fieldOwner,
948-
MaxRetryDuration: time.Minute,
949-
EventSource: kubeutil.NewEventStore(),
950-
Client: fakeClient,
951-
Sign: tc.sign,
952-
EventRecorder: fakeRecorder,
953-
Clock: fakeClock2,
946+
IssuerTypes: map[schema.GroupResource]v1alpha1.Issuer{
947+
api.TestIssuerGroupVersionResource.GroupResource(): &api.TestIssuer{},
948+
},
949+
ClusterIssuerTypes: map[schema.GroupResource]v1alpha1.Issuer{
950+
api.TestClusterIssuerGroupVersionResource.GroupResource(): &api.TestClusterIssuer{},
951+
},
952+
FieldOwner: fieldOwner,
953+
MaxRetryDuration: time.Minute,
954+
EventSource: kubeutil.NewEventStore(),
955+
Client: fakeClient,
956+
Sign: tc.sign,
957+
EventRecorder: fakeRecorder,
958+
Clock: fakeClock2,
954959
},
955960
}).Init()
956961

@@ -1000,8 +1005,8 @@ func TestCertificateRequestMatchIssuerType(t *testing.T) {
10001005
type testcase struct {
10011006
name string
10021007

1003-
issuerTypes []v1alpha1.Issuer
1004-
clusterIssuerTypes []v1alpha1.Issuer
1008+
issuerTypes map[schema.GroupResource]v1alpha1.Issuer
1009+
clusterIssuerTypes map[schema.GroupResource]v1alpha1.Issuer
10051010
cr *cmapi.CertificateRequest
10061011

10071012
expectedIssuerType v1alpha1.Issuer
@@ -1046,46 +1051,64 @@ func TestCertificateRequestMatchIssuerType(t *testing.T) {
10461051
expectedError: errormatch.ErrorContains("no issuer found for reference: [Group=\"test\", Kind=\"\", Name=\"name\"]"),
10471052
},
10481053
{
1049-
name: "match issuer",
1050-
issuerTypes: []v1alpha1.Issuer{&api.TestIssuer{}},
1051-
clusterIssuerTypes: []v1alpha1.Issuer{&api.TestClusterIssuer{}},
1052-
cr: createCr("name", "namespace", "TestIssuer", "testing.cert-manager.io"),
1054+
name: "match issuer",
1055+
issuerTypes: map[schema.GroupResource]v1alpha1.Issuer{
1056+
api.TestIssuerGroupVersionResource.GroupResource(): &api.TestIssuer{},
1057+
},
1058+
clusterIssuerTypes: map[schema.GroupResource]v1alpha1.Issuer{
1059+
api.TestClusterIssuerGroupVersionResource.GroupResource(): &api.TestClusterIssuer{},
1060+
},
1061+
cr: createCr("name", "namespace", "TestIssuer", "testing.cert-manager.io"),
10531062

10541063
expectedIssuerType: &api.TestIssuer{},
10551064
expectedIssuerName: types.NamespacedName{Name: "name", Namespace: "namespace"},
10561065
},
10571066
{
1058-
name: "match cluster issuer",
1059-
issuerTypes: []v1alpha1.Issuer{&api.TestIssuer{}},
1060-
clusterIssuerTypes: []v1alpha1.Issuer{&api.TestClusterIssuer{}},
1061-
cr: createCr("name", "namespace", "TestClusterIssuer", "testing.cert-manager.io"),
1067+
name: "match cluster issuer",
1068+
issuerTypes: map[schema.GroupResource]v1alpha1.Issuer{
1069+
api.TestIssuerGroupVersionResource.GroupResource(): &api.TestIssuer{},
1070+
},
1071+
clusterIssuerTypes: map[schema.GroupResource]v1alpha1.Issuer{
1072+
api.TestClusterIssuerGroupVersionResource.GroupResource(): &api.TestClusterIssuer{},
1073+
},
1074+
cr: createCr("name", "namespace", "TestClusterIssuer", "testing.cert-manager.io"),
10621075

10631076
expectedIssuerType: &api.TestClusterIssuer{},
10641077
expectedIssuerName: types.NamespacedName{Name: "name"},
10651078
},
10661079
{
1067-
name: "select kind if empty",
1068-
issuerTypes: []v1alpha1.Issuer{},
1069-
clusterIssuerTypes: []v1alpha1.Issuer{&api.TestClusterIssuer{}},
1070-
cr: createCr("name", "namespace", "", "testing.cert-manager.io"),
1080+
name: "select kind if empty",
1081+
issuerTypes: map[schema.GroupResource]v1alpha1.Issuer{},
1082+
clusterIssuerTypes: map[schema.GroupResource]v1alpha1.Issuer{
1083+
api.TestClusterIssuerGroupVersionResource.GroupResource(): &api.TestClusterIssuer{},
1084+
},
1085+
cr: createCr("name", "namespace", "", "testing.cert-manager.io"),
10711086

10721087
expectedIssuerType: &api.TestClusterIssuer{},
10731088
expectedIssuerName: types.NamespacedName{Name: "name"},
10741089
},
10751090
{
1076-
name: "prefer issuer over cluster issuer (v1)",
1077-
issuerTypes: []v1alpha1.Issuer{&api.TestIssuer{}},
1078-
clusterIssuerTypes: []v1alpha1.Issuer{&api.TestClusterIssuer{}},
1079-
cr: createCr("name", "namespace", "", "testing.cert-manager.io"),
1091+
name: "prefer issuer over cluster issuer (v1)",
1092+
issuerTypes: map[schema.GroupResource]v1alpha1.Issuer{
1093+
api.TestIssuerGroupVersionResource.GroupResource(): &api.TestIssuer{},
1094+
},
1095+
clusterIssuerTypes: map[schema.GroupResource]v1alpha1.Issuer{
1096+
api.TestClusterIssuerGroupVersionResource.GroupResource(): &api.TestClusterIssuer{},
1097+
},
1098+
cr: createCr("name", "namespace", "", "testing.cert-manager.io"),
10801099

10811100
expectedIssuerType: &api.TestIssuer{},
10821101
expectedIssuerName: types.NamespacedName{Name: "name", Namespace: "namespace"},
10831102
},
10841103
{
1085-
name: "prefer issuer over cluster issuer (v2)",
1086-
issuerTypes: []v1alpha1.Issuer{&api.TestIssuer{}},
1087-
clusterIssuerTypes: []v1alpha1.Issuer{&api.TestIssuer{}},
1088-
cr: createCr("name", "namespace", "", "testing.cert-manager.io"),
1104+
name: "prefer issuer over cluster issuer (v2)",
1105+
issuerTypes: map[schema.GroupResource]v1alpha1.Issuer{
1106+
api.TestIssuerGroupVersionResource.GroupResource(): &api.TestIssuer{},
1107+
},
1108+
clusterIssuerTypes: map[schema.GroupResource]v1alpha1.Issuer{
1109+
api.TestIssuerGroupVersionResource.GroupResource(): &api.TestIssuer{},
1110+
},
1111+
cr: createCr("name", "namespace", "", "testing.cert-manager.io"),
10891112

10901113
expectedIssuerType: &api.TestIssuer{},
10911114
expectedIssuerName: types.NamespacedName{Name: "name", Namespace: "namespace"},

‎controllers/certificatesigningrequest_controller.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ func (r *CertificateSigningRequestReconciler) matchIssuerType(requestObject clie
5959

6060
// Search for matching issuer
6161
for _, issuerType := range r.AllIssuerTypes() {
62-
if issuerTypeIdentifier != issuerType.Type.GetIssuerTypeIdentifier() {
62+
if issuerTypeIdentifier != issuerType.IssuerTypeIdentifier {
6363
continue
6464
}
6565

0 commit comments

Comments
 (0)