Skip to content

Commit 86ed14d

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 3aef2fa commit 86ed14d

18 files changed

+406
-318
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"
@@ -85,12 +86,16 @@ func TestCertificateRequestControllerIntegrationIssuerInitiallyNotFoundAndNotRea
8586
func(mgr ctrl.Manager) controllerInterface {
8687
return &CertificateRequestReconciler{
8788
RequestController: RequestController{
88-
IssuerTypes: []v1alpha1.Issuer{&api.TestIssuer{}},
89-
ClusterIssuerTypes: []v1alpha1.Issuer{&api.TestClusterIssuer{}},
90-
FieldOwner: fieldOwner,
91-
MaxRetryDuration: time.Minute,
92-
EventSource: kubeutil.NewEventStore(),
93-
Client: mgr.GetClient(),
89+
IssuerTypes: map[schema.GroupResource]v1alpha1.Issuer{
90+
api.TestIssuerGroupVersionResource.GroupResource(): &api.TestIssuer{},
91+
},
92+
ClusterIssuerTypes: map[schema.GroupResource]v1alpha1.Issuer{
93+
api.TestClusterIssuerGroupVersionResource.GroupResource(): &api.TestClusterIssuer{},
94+
},
95+
FieldOwner: fieldOwner,
96+
MaxRetryDuration: time.Minute,
97+
EventSource: kubeutil.NewEventStore(),
98+
Client: mgr.GetClient(),
9499
Sign: func(_ context.Context, cr signer.CertificateRequestObject, _ v1alpha1.Issuer) (signer.PEMBundle, error) {
95100
atomic.AddUint64(&counters[extractIdFromNamespace(t, cr.GetNamespace())], 1)
96101
return signer.PEMBundle{
@@ -224,12 +229,16 @@ func TestCertificateRequestControllerIntegrationSetCondition(t *testing.T) {
224229
func(mgr ctrl.Manager) controllerInterface {
225230
return &CertificateRequestReconciler{
226231
RequestController: RequestController{
227-
IssuerTypes: []v1alpha1.Issuer{&api.TestIssuer{}},
228-
ClusterIssuerTypes: []v1alpha1.Issuer{&api.TestClusterIssuer{}},
229-
FieldOwner: fieldOwner,
230-
MaxRetryDuration: time.Minute,
231-
EventSource: kubeutil.NewEventStore(),
232-
Client: mgr.GetClient(),
232+
IssuerTypes: map[schema.GroupResource]v1alpha1.Issuer{
233+
api.TestIssuerGroupVersionResource.GroupResource(): &api.TestIssuer{},
234+
},
235+
ClusterIssuerTypes: map[schema.GroupResource]v1alpha1.Issuer{
236+
api.TestClusterIssuerGroupVersionResource.GroupResource(): &api.TestClusterIssuer{},
237+
},
238+
FieldOwner: fieldOwner,
239+
MaxRetryDuration: time.Minute,
240+
EventSource: kubeutil.NewEventStore(),
241+
Client: mgr.GetClient(),
233242
Sign: func(ctx context.Context, cr signer.CertificateRequestObject, _ v1alpha1.Issuer) (signer.PEMBundle, error) {
234243
atomic.AddUint64(&counter, 1)
235244
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)