Skip to content

Commit 9c5cde5

Browse files
yevgeny-shnaidmanyevgeny-shnaidman
authored
Moving authenticator creation to auth package (#42) (kubernetes-sigs#61)
1 parent 90b75fa commit 9c5cde5

File tree

9 files changed

+71
-105
lines changed

9 files changed

+71
-105
lines changed

‎controllers/module_reconciler.go

+1-20
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@ import (
2424
kmmv1beta1 "github.com/rh-ecosystem-edge/kernel-module-management/api/v1beta1"
2525
"github.com/rh-ecosystem-edge/kernel-module-management/internal/auth"
2626
"github.com/rh-ecosystem-edge/kernel-module-management/internal/build"
27-
"github.com/rh-ecosystem-edge/kernel-module-management/internal/constants"
2827
"github.com/rh-ecosystem-edge/kernel-module-management/internal/daemonset"
2928
"github.com/rh-ecosystem-edge/kernel-module-management/internal/filter"
3029
"github.com/rh-ecosystem-edge/kernel-module-management/internal/metrics"
@@ -38,7 +37,6 @@ import (
3837
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
3938
"k8s.io/apimachinery/pkg/types"
4039
"k8s.io/apimachinery/pkg/util/sets"
41-
"k8s.io/client-go/kubernetes"
4240
ctrl "sigs.k8s.io/controller-runtime"
4341
"sigs.k8s.io/controller-runtime/pkg/builder"
4442
"sigs.k8s.io/controller-runtime/pkg/client"
@@ -54,7 +52,6 @@ type ModuleReconciler struct {
5452

5553
authFactory auth.RegistryAuthGetterFactory
5654
buildAPI build.Manager
57-
coreClientSet kubernetes.Interface
5855
daemonAPI daemonset.DaemonSetCreator
5956
kernelAPI module.KernelMapper
6057
metricsAPI metrics.Metrics
@@ -65,7 +62,6 @@ type ModuleReconciler struct {
6562

6663
func NewModuleReconciler(
6764
client client.Client,
68-
coreClientSet kubernetes.Interface,
6965
buildAPI build.Manager,
7066
daemonAPI daemonset.DaemonSetCreator,
7167
kernelAPI module.KernelMapper,
@@ -78,7 +74,6 @@ func NewModuleReconciler(
7874
Client: client,
7975
authFactory: authFactory,
8076
buildAPI: buildAPI,
81-
coreClientSet: coreClientSet,
8277
daemonAPI: daemonAPI,
8378
kernelAPI: kernelAPI,
8479
metricsAPI: metricsAPI,
@@ -275,21 +270,7 @@ func (r *ModuleReconciler) handleBuild(ctx context.Context,
275270
}
276271

277272
func (r *ModuleReconciler) checkImageExists(ctx context.Context, mod *kmmv1beta1.Module, km *kmmv1beta1.KernelMapping) (bool, error) {
278-
var registryAuthGetter auth.RegistryAuthGetter
279-
280-
if irs := mod.Spec.ImageRepoSecret; irs != nil {
281-
namespacedName := types.NamespacedName{
282-
Name: irs.Name,
283-
Namespace: mod.Namespace,
284-
}
285-
registryAuthGetter = r.authFactory.NewRegistryAuthGetter(r.Client, namespacedName)
286-
} else {
287-
registryAuthGetter = r.authFactory.NewServiceAccountRegistryAuthGetter(
288-
r.coreClientSet,
289-
mod.Namespace,
290-
constants.OCPBuilderServiceAccountName)
291-
}
292-
273+
registryAuthGetter := r.authFactory.NewRegistryAuthGetterFrom(mod)
293274
pullOptions := module.GetRelevantPullOptions(mod, km)
294275
imageAvailable, err := r.registry.ImageExists(ctx, km.ContainerImage, pullOptions, registryAuthGetter)
295276
if err != nil {

‎controllers/module_reconciler_test.go

+5-20
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ import (
1010
"github.com/rh-ecosystem-edge/kernel-module-management/internal/auth"
1111
"github.com/rh-ecosystem-edge/kernel-module-management/internal/build"
1212
"github.com/rh-ecosystem-edge/kernel-module-management/internal/client"
13-
"github.com/rh-ecosystem-edge/kernel-module-management/internal/constants"
1413
"github.com/rh-ecosystem-edge/kernel-module-management/internal/daemonset"
1514
"github.com/rh-ecosystem-edge/kernel-module-management/internal/metrics"
1615
"github.com/rh-ecosystem-edge/kernel-module-management/internal/module"
@@ -23,7 +22,6 @@ import (
2322
"k8s.io/apimachinery/pkg/runtime/schema"
2423
"k8s.io/apimachinery/pkg/types"
2524
"k8s.io/apimachinery/pkg/util/sets"
26-
"k8s.io/client-go/kubernetes/fake"
2725
"sigs.k8s.io/controller-runtime/pkg/reconcile"
2826
)
2927

@@ -75,7 +73,6 @@ var _ = Describe("ModuleReconciler_Reconcile", func() {
7573

7674
mr := NewModuleReconciler(
7775
clnt,
78-
nil,
7976
mockBM,
8077
mockDC,
8178
mockKM,
@@ -128,7 +125,6 @@ var _ = Describe("ModuleReconciler_Reconcile", func() {
128125

129126
mr := NewModuleReconciler(
130127
clnt,
131-
nil,
132128
mockBM,
133129
mockDC,
134130
mockKM,
@@ -197,7 +193,6 @@ var _ = Describe("ModuleReconciler_Reconcile", func() {
197193

198194
mr := NewModuleReconciler(
199195
clnt,
200-
nil,
201196
mockBM,
202197
mockDC,
203198
mockKM,
@@ -269,7 +264,6 @@ var _ = Describe("ModuleReconciler_Reconcile", func() {
269264

270265
mr := NewModuleReconciler(
271266
clnt,
272-
nil,
273267
mockBM,
274268
mockDC,
275269
mockKM,
@@ -409,7 +403,6 @@ var _ = Describe("ModuleReconciler_Reconcile", func() {
409403

410404
mr := NewModuleReconciler(
411405
clnt,
412-
nil,
413406
mockBM,
414407
mockDC,
415408
mockKM,
@@ -471,7 +464,6 @@ var _ = Describe("ModuleReconciler_Reconcile", func() {
471464

472465
mr := NewModuleReconciler(
473466
clnt,
474-
nil,
475467
mockBM,
476468
mockDC,
477469
mockKM,
@@ -530,7 +522,6 @@ var _ = Describe("ModuleReconciler_handleBuild", func() {
530522
authGetter = &auth.MockRegistryAuthGetter{}
531523
ctrl *gomock.Controller
532524
clnt *client.MockClient
533-
fakeClientSet = fake.NewSimpleClientset()
534525
mockAuthFactory *auth.MockRegistryAuthGetterFactory
535526
mockBM *build.MockManager
536527
mockDC *daemonset.MockDaemonSetCreator
@@ -568,7 +559,6 @@ var _ = Describe("ModuleReconciler_handleBuild", func() {
568559

569560
mr := NewModuleReconciler(
570561
clnt,
571-
nil,
572562
mockBM,
573563
mockDC,
574564
mockKM,
@@ -607,7 +597,7 @@ var _ = Describe("ModuleReconciler_handleBuild", func() {
607597
gomock.InOrder(
608598
mockAuthFactory.
609599
EXPECT().
610-
NewRegistryAuthGetter(clnt, types.NamespacedName{Name: secretName, Namespace: namespace}).
600+
NewRegistryAuthGetterFrom(mod).
611601
Return(authGetter),
612602
mockRegistry.EXPECT().ImageExists(context.Background(), imageName, nil, authGetter).DoAndReturn(
613603
func(_ interface{}, _ interface{}, _ interface{}, registryAuthGetter auth.RegistryAuthGetter) (bool, error) {
@@ -619,7 +609,6 @@ var _ = Describe("ModuleReconciler_handleBuild", func() {
619609

620610
mr := NewModuleReconciler(
621611
clnt,
622-
fakeClientSet,
623612
mockBM,
624613
mockDC,
625614
mockKM,
@@ -651,7 +640,7 @@ var _ = Describe("ModuleReconciler_handleBuild", func() {
651640
gomock.InOrder(
652641
mockAuthFactory.
653642
EXPECT().
654-
NewServiceAccountRegistryAuthGetter(fakeClientSet, namespace, constants.OCPBuilderServiceAccountName).
643+
NewRegistryAuthGetterFrom(mod).
655644
Return(authGetter),
656645
mockRegistry.EXPECT().ImageExists(context.Background(), imageName, nil, authGetter).DoAndReturn(
657646
func(_ interface{}, _ interface{}, _ interface{}, registryAuthGetter auth.RegistryAuthGetter) (bool, error) {
@@ -663,7 +652,6 @@ var _ = Describe("ModuleReconciler_handleBuild", func() {
663652

664653
mr := NewModuleReconciler(
665654
clnt,
666-
fakeClientSet,
667655
mockBM,
668656
mockDC,
669657
mockKM,
@@ -696,7 +684,7 @@ var _ = Describe("ModuleReconciler_handleBuild", func() {
696684
gomock.InOrder(
697685
mockAuthFactory.
698686
EXPECT().
699-
NewServiceAccountRegistryAuthGetter(fakeClientSet, namespace, constants.OCPBuilderServiceAccountName).
687+
NewRegistryAuthGetterFrom(mod).
700688
Return(authGetter),
701689
mockRegistry.EXPECT().ImageExists(context.Background(), imageName, nil, authGetter).Return(false, nil),
702690
mockBM.EXPECT().Sync(gomock.Any(), *mod, *km, gomock.Any()).Return(buildRes, nil),
@@ -705,7 +693,6 @@ var _ = Describe("ModuleReconciler_handleBuild", func() {
705693

706694
mr := NewModuleReconciler(
707695
clnt,
708-
fakeClientSet,
709696
mockBM,
710697
mockDC,
711698
mockKM,
@@ -738,7 +725,7 @@ var _ = Describe("ModuleReconciler_handleBuild", func() {
738725
gomock.InOrder(
739726
mockAuthFactory.
740727
EXPECT().
741-
NewServiceAccountRegistryAuthGetter(fakeClientSet, namespace, constants.OCPBuilderServiceAccountName).
728+
NewRegistryAuthGetterFrom(mod).
742729
Return(authGetter),
743730
mockRegistry.EXPECT().ImageExists(context.Background(), imageName, nil, authGetter).Return(false, nil),
744731
mockBM.EXPECT().Sync(gomock.Any(), *mod, *km, gomock.Any()).Return(buildRes, nil),
@@ -747,7 +734,6 @@ var _ = Describe("ModuleReconciler_handleBuild", func() {
747734

748735
mr := NewModuleReconciler(
749736
clnt,
750-
fakeClientSet,
751737
mockBM,
752738
mockDC,
753739
mockKM,
@@ -780,7 +766,7 @@ var _ = Describe("ModuleReconciler_handleBuild", func() {
780766
gomock.InOrder(
781767
mockAuthFactory.
782768
EXPECT().
783-
NewServiceAccountRegistryAuthGetter(fakeClientSet, namespace, constants.OCPBuilderServiceAccountName).
769+
NewRegistryAuthGetterFrom(mod).
784770
Return(authGetter),
785771
mockRegistry.EXPECT().ImageExists(context.Background(), imageName, nil, authGetter).Return(false, nil),
786772
mockBM.EXPECT().Sync(gomock.Any(), *mod, *km, gomock.Any()).Return(buildRes, nil),
@@ -789,7 +775,6 @@ var _ = Describe("ModuleReconciler_handleBuild", func() {
789775

790776
mr := NewModuleReconciler(
791777
clnt,
792-
fakeClientSet,
793778
mockBM,
794779
mockDC,
795780
mockKM,

‎internal/auth/auth.go

+29-9
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ import (
66

77
"github.com/google/go-containerregistry/pkg/authn"
88
"github.com/google/go-containerregistry/pkg/authn/kubernetes"
9+
kmmv1beta1 "github.com/rh-ecosystem-edge/kernel-module-management/api/v1beta1"
10+
"github.com/rh-ecosystem-edge/kernel-module-management/internal/constants"
911
v1 "k8s.io/api/core/v1"
1012
"k8s.io/apimachinery/pkg/types"
1113
k8s "k8s.io/client-go/kubernetes"
@@ -59,27 +61,45 @@ func (sarag *serviceAccountRegistryAuthGetter) GetKeyChain(ctx context.Context)
5961
}
6062

6163
type RegistryAuthGetterFactory interface {
62-
NewRegistryAuthGetter(client client.Client, namespacedName types.NamespacedName) RegistryAuthGetter
63-
NewServiceAccountRegistryAuthGetter(coreClientSet k8s.Interface, namespace, serviceAccountName string) RegistryAuthGetter
64+
NewRegistryAuthGetterFrom(mod *kmmv1beta1.Module) RegistryAuthGetter
6465
}
6566

66-
type registryAuthGetterFactory struct{}
67+
type registryAuthGetterFactory struct {
68+
client client.Client
69+
coreClientSet k8s.Interface
70+
}
6771

68-
func NewRegistryAuthGetterFactory() RegistryAuthGetterFactory {
69-
return registryAuthGetterFactory{}
72+
func NewRegistryAuthGetterFactory(client client.Client, coreClientSet k8s.Interface) RegistryAuthGetterFactory {
73+
return &registryAuthGetterFactory{
74+
client: client,
75+
coreClientSet: coreClientSet,
76+
}
7077
}
7178

72-
func (registryAuthGetterFactory) NewRegistryAuthGetter(client client.Client, namespacedName types.NamespacedName) RegistryAuthGetter {
79+
func (af *registryAuthGetterFactory) newRegistryAuthGetter(namespacedName types.NamespacedName) RegistryAuthGetter {
7380
return &registrySecretAuthGetter{
74-
client: client,
81+
client: af.client,
7582
namespacedName: namespacedName,
7683
}
7784
}
7885

79-
func (registryAuthGetterFactory) NewServiceAccountRegistryAuthGetter(coreClientSet k8s.Interface, namespace, serviceAccountName string) RegistryAuthGetter {
86+
func (af *registryAuthGetterFactory) newServiceAccountRegistryAuthGetter(namespace, serviceAccountName string) RegistryAuthGetter {
8087
return &serviceAccountRegistryAuthGetter{
81-
coreClientSet: coreClientSet,
88+
coreClientSet: af.coreClientSet,
8289
namespace: namespace,
8390
serviceAccountName: serviceAccountName,
8491
}
8592
}
93+
94+
func (af *registryAuthGetterFactory) NewRegistryAuthGetterFrom(mod *kmmv1beta1.Module) RegistryAuthGetter {
95+
if mod.Spec.ImageRepoSecret != nil {
96+
namespacedName := types.NamespacedName{
97+
Name: mod.Spec.ImageRepoSecret.Name,
98+
Namespace: mod.Namespace,
99+
}
100+
return af.newRegistryAuthGetter(namespacedName)
101+
}
102+
return af.newServiceAccountRegistryAuthGetter(
103+
mod.Namespace,
104+
constants.OCPBuilderServiceAccountName)
105+
}

‎internal/auth/auth_test.go

+12-7
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@ import (
1010
"github.com/rh-ecosystem-edge/kernel-module-management/internal/client"
1111
v1 "k8s.io/api/core/v1"
1212
"k8s.io/apimachinery/pkg/types"
13+
"k8s.io/client-go/kubernetes"
14+
"k8s.io/client-go/kubernetes/fake"
1315
)
1416

1517
var _ = Describe("registrySecretAuthGetter_GetKeyChain", func() {
@@ -19,16 +21,19 @@ var _ = Describe("registrySecretAuthGetter_GetKeyChain", func() {
1921
)
2022

2123
var (
22-
ctrl *gomock.Controller
23-
ctx context.Context
24-
factory = NewRegistryAuthGetterFactory()
25-
mockClient *client.MockClient
24+
ctrl *gomock.Controller
25+
ctx context.Context
26+
factory *registryAuthGetterFactory
27+
mockClient *client.MockClient
28+
fakeClientSet kubernetes.Interface
2629
)
2730

2831
BeforeEach(func() {
2932
ctrl = gomock.NewController(GinkgoT())
3033
ctx = context.TODO()
3134
mockClient = client.NewMockClient(ctrl)
35+
fakeClientSet = fake.NewSimpleClientset()
36+
factory = NewRegistryAuthGetterFactory(mockClient, fakeClientSet).(*registryAuthGetterFactory)
3237
})
3338

3439
AfterEach(func() {
@@ -43,7 +48,7 @@ var _ = Describe("registrySecretAuthGetter_GetKeyChain", func() {
4348
Name: secretName,
4449
Namespace: secretNamespace,
4550
}
46-
registryAuthGetter := factory.NewRegistryAuthGetter(mockClient, namespacedNamespace)
51+
registryAuthGetter := factory.newRegistryAuthGetter(namespacedNamespace)
4752

4853
_, err := registryAuthGetter.GetKeyChain(ctx)
4954
Expect(err).To(HaveOccurred())
@@ -66,7 +71,7 @@ var _ = Describe("registrySecretAuthGetter_GetKeyChain", func() {
6671
Name: secretName,
6772
Namespace: secretNamespace,
6873
}
69-
registryAuthGetter := factory.NewRegistryAuthGetter(mockClient, namespacedNamespace)
74+
registryAuthGetter := factory.newRegistryAuthGetter(namespacedNamespace)
7075

7176
_, err := registryAuthGetter.GetKeyChain(ctx)
7277
Expect(err).To(HaveOccurred())
@@ -81,7 +86,7 @@ var _ = Describe("registrySecretAuthGetter_GetKeyChain", func() {
8186
Name: secretName,
8287
Namespace: secretNamespace,
8388
}
84-
registryAuthGetter := factory.NewRegistryAuthGetter(mockClient, namespacedNamespace)
89+
registryAuthGetter := factory.newRegistryAuthGetter(namespacedNamespace)
8590

8691
_, err := registryAuthGetter.GetKeyChain(ctx)
8792
Expect(err).NotTo(HaveOccurred())

‎internal/auth/mock_auth.go

+7-23
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)