diff --git a/main.go b/main.go index 0dafdcc06..238fbf005 100644 --- a/main.go +++ b/main.go @@ -126,7 +126,13 @@ func main() { proxyAPI, resourcehelper.New()) - registryAPI := registry.NewRegistry(kubeClient, registry.NewCraneWrapper(kubeClient, registry.RegistryConfFilePath)) + craneWrapperAPI := registry.NewCraneWrapper( + kubeClient, + registry.NewOpenShiftCAGetter(kubeClient), + registry.RegistryConfFilePath, + ) + + registryAPI := registry.NewRegistry(kubeClient, craneWrapperAPI) clusterInfoAPI := upgrade.NewClusterInfo(registryAPI, clusterAPI) runtimeAPI := runtime.NewRuntimeAPI(kubeClient, clusterAPI, kernelAPI, clusterInfoAPI, proxyAPI) diff --git a/pkg/registry/mock_openshift_ca_api.go b/pkg/registry/mock_openshift_ca_api.go new file mode 100644 index 000000000..dcd0aaebe --- /dev/null +++ b/pkg/registry/mock_openshift_ca_api.go @@ -0,0 +1,65 @@ +// Code generated by MockGen. DO NOT EDIT. +// Source: openshift_ca.go + +// Package registry is a generated GoMock package. +package registry + +import ( + context "context" + reflect "reflect" + + gomock "github.com/golang/mock/gomock" +) + +// MockOpenShiftCAGetter is a mock of OpenShiftCAGetter interface. +type MockOpenShiftCAGetter struct { + ctrl *gomock.Controller + recorder *MockOpenShiftCAGetterMockRecorder +} + +// MockOpenShiftCAGetterMockRecorder is the mock recorder for MockOpenShiftCAGetter. +type MockOpenShiftCAGetterMockRecorder struct { + mock *MockOpenShiftCAGetter +} + +// NewMockOpenShiftCAGetter creates a new mock instance. +func NewMockOpenShiftCAGetter(ctrl *gomock.Controller) *MockOpenShiftCAGetter { + mock := &MockOpenShiftCAGetter{ctrl: ctrl} + mock.recorder = &MockOpenShiftCAGetterMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockOpenShiftCAGetter) EXPECT() *MockOpenShiftCAGetterMockRecorder { + return m.recorder +} + +// AdditionalTrustedCAs mocks base method. +func (m *MockOpenShiftCAGetter) AdditionalTrustedCAs(ctx context.Context) (map[string][]byte, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "AdditionalTrustedCAs", ctx) + ret0, _ := ret[0].(map[string][]byte) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// AdditionalTrustedCAs indicates an expected call of AdditionalTrustedCAs. +func (mr *MockOpenShiftCAGetterMockRecorder) AdditionalTrustedCAs(ctx interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AdditionalTrustedCAs", reflect.TypeOf((*MockOpenShiftCAGetter)(nil).AdditionalTrustedCAs), ctx) +} + +// CABundle mocks base method. +func (m *MockOpenShiftCAGetter) CABundle(arg0 context.Context) ([]byte, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "CABundle", arg0) + ret0, _ := ret[0].([]byte) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// CABundle indicates an expected call of CABundle. +func (mr *MockOpenShiftCAGetterMockRecorder) CABundle(arg0 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CABundle", reflect.TypeOf((*MockOpenShiftCAGetter)(nil).CABundle), arg0) +} diff --git a/pkg/registry/openshift_ca.go b/pkg/registry/openshift_ca.go new file mode 100644 index 000000000..78cab5b76 --- /dev/null +++ b/pkg/registry/openshift_ca.go @@ -0,0 +1,87 @@ +package registry + +import ( + "context" + "fmt" + + "github.com/openshift/special-resource-operator/pkg/clients" + k8serrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + ctrl "sigs.k8s.io/controller-runtime" +) + +type openShiftCAGetter struct { + kubeClient clients.ClientsInterface +} + +//go:generate mockgen -source=openshift_ca.go -package=registry -destination=mock_openshift_ca_api.go + +type OpenShiftCAGetter interface { + AdditionalTrustedCAs(ctx context.Context) (map[string][]byte, error) + CABundle(ctx context.Context) ([]byte, error) +} + +func NewOpenShiftCAGetter(kubeClient clients.ClientsInterface) OpenShiftCAGetter { + return &openShiftCAGetter{kubeClient: kubeClient} +} + +func (ocg *openShiftCAGetter) AdditionalTrustedCAs(ctx context.Context) (map[string][]byte, error) { + logger := ctrl.LoggerFrom(ctx) + + var certs map[string][]byte + + logger.Info("Getting image config", "name", imageClusterName) + + img, err := ocg.kubeClient.GetImage(ctx, imageClusterName, metav1.GetOptions{}) + if err != nil { + return nil, fmt.Errorf("could not retrieve image.config.openshift.io/%s: %w", imageClusterName, err) + } + + if cmName := img.Spec.AdditionalTrustedCA.Name; cmName != "" { + logger.Info("Getting ConfigMap", "cm-name", cmName, "cm-namespace", configNamespace) + + cm, err := ocg.kubeClient.GetConfigMap(ctx, configNamespace, cmName, metav1.GetOptions{}) + if err != nil { + return nil, fmt.Errorf("could not retrieve configmap %s/%s: %w", configNamespace, cmName, err) + } + + certs = make(map[string][]byte, len(cm.Data)) + + for registry, cert := range cm.Data { + certs[registry] = []byte(cert) + } + } + + return certs, nil + +} + +func (ocg *openShiftCAGetter) CABundle(ctx context.Context) ([]byte, error) { + logger := ctrl.LoggerFrom(ctx) + + const userCABundleCMName = "user-ca-bundle" + + cmLogger := logger.WithValues("cm-name", userCABundleCMName, "cm-namespace", configNamespace) + + cmLogger.Info("Getting ConfigMap") + + userCABundle, err := ocg.kubeClient.GetConfigMap(ctx, configNamespace, userCABundleCMName, metav1.GetOptions{}) + if err != nil { + // It's okay if that ConfigMap does not exist. + if !k8serrors.IsNotFound(err) { + return nil, fmt.Errorf("could not get ConfigMap %s/%s: %v", configNamespace, userCABundleCMName, err) + } + + cmLogger.Info("No such ConfigMap; skipping") + return nil, nil + } + + const caBundleKey = "ca-bundle.crt" + + if data, ok := userCABundle.Data[caBundleKey]; ok { + return []byte(data), nil + } + + cmLogger.Info("No such key; skipping", "key", caBundleKey) + return nil, nil +} diff --git a/pkg/registry/openshift_ca_test.go b/pkg/registry/openshift_ca_test.go new file mode 100644 index 000000000..951a2fa2b --- /dev/null +++ b/pkg/registry/openshift_ca_test.go @@ -0,0 +1,172 @@ +package registry + +import ( + "context" + "errors" + + "github.com/golang/mock/gomock" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + configv1 "github.com/openshift/api/config/v1" + "github.com/openshift/special-resource-operator/pkg/clients" + v1 "k8s.io/api/core/v1" + k8serrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" +) + +var _ = Describe("OpenShiftCAGetter", func() { + var kubeClient *clients.MockClientsInterface + + BeforeEach(func() { + ctrl := gomock.NewController(GinkgoT()) + kubeClient = clients.NewMockClientsInterface(ctrl) + }) + + Describe("AdditionalTrustedCAs", func() { + const imageConfigName = "cluster" + + It("should return an error if we cannot get the image.config.openshift.io", func() { + kubeClient. + EXPECT(). + GetImage(context.Background(), imageConfigName, metav1.GetOptions{}). + Return(nil, errors.New("random error")) + + _, err := NewOpenShiftCAGetter(kubeClient).AdditionalTrustedCAs(context.Background()) + Expect(err).To(HaveOccurred()) + }) + + It("should return an empty slice if the additionalTrustedCA is empty", func() { + kubeClient. + EXPECT(). + GetImage(context.Background(), imageConfigName, metav1.GetOptions{}). + Return(&configv1.Image{}, nil) + + c, err := NewOpenShiftCAGetter(kubeClient).AdditionalTrustedCAs(context.Background()) + Expect(err).NotTo(HaveOccurred()) + Expect(c).To(BeEmpty()) + }) + + It("should an empty slice if the configmap is empty", func() { + const cmName = "cm-name" + + img := configv1.Image{ + Spec: configv1.ImageSpec{ + AdditionalTrustedCA: configv1.ConfigMapNameReference{Name: cmName}, + }, + } + + gomock.InOrder( + kubeClient. + EXPECT(). + GetImage(context.Background(), imageConfigName, metav1.GetOptions{}). + Return(&img, nil), + kubeClient. + EXPECT(). + GetConfigMap(context.Background(), "openshift-config", cmName, metav1.GetOptions{}). + Return(&v1.ConfigMap{}, nil), + ) + + c, err := NewOpenShiftCAGetter(kubeClient).AdditionalTrustedCAs(context.Background()) + Expect(err).NotTo(HaveOccurred()) + Expect(c).To(BeEmpty()) + }) + + It("should work as expected", func() { + const cmName = "cm-name" + + img := configv1.Image{ + Spec: configv1.ImageSpec{ + AdditionalTrustedCA: configv1.ConfigMapNameReference{Name: cmName}, + }, + } + + const ( + cmKey = "key" + cmValue = "value" + ) + + cm := v1.ConfigMap{ + Data: map[string]string{cmKey: cmValue}, + } + + gomock.InOrder( + kubeClient. + EXPECT(). + GetImage(context.Background(), imageConfigName, metav1.GetOptions{}). + Return(&img, nil), + kubeClient. + EXPECT(). + GetConfigMap(context.Background(), "openshift-config", cmName, metav1.GetOptions{}). + Return(&cm, nil), + ) + + c, err := NewOpenShiftCAGetter(kubeClient).AdditionalTrustedCAs(context.Background()) + Expect(err).NotTo(HaveOccurred()) + Expect(c).To( + Equal( + map[string][]byte{cmKey: []byte(cmValue)}, + ), + ) + }) + }) + + Describe("CABundle", func() { + const cmName = "user-ca-bundle" + + It("should return an error if we cannot fetch the ConfigMap", func() { + randomError := errors.New("random error") + + kubeClient. + EXPECT(). + GetConfigMap(context.Background(), "openshift-config", cmName, metav1.GetOptions{}). + Return(nil, randomError) + + _, err := NewOpenShiftCAGetter(kubeClient).CABundle(context.Background()) + Expect(err).To(HaveOccurred()) + }) + + It("should return an empty slice if the ConfigMap does not exist", func() { + notFound := k8serrors.NewNotFound(schema.GroupResource{}, cmName) + + kubeClient. + EXPECT(). + GetConfigMap(context.Background(), "openshift-config", cmName, metav1.GetOptions{}). + Return(nil, notFound) + + s, err := NewOpenShiftCAGetter(kubeClient).CABundle(context.Background()) + Expect(err).NotTo(HaveOccurred()) + Expect(s).To(BeEmpty()) + }) + + It("should return an empty slice if the ConfigMap does not have the expected key", func() { + kubeClient. + EXPECT(). + GetConfigMap(context.Background(), "openshift-config", cmName, metav1.GetOptions{}). + Return(&v1.ConfigMap{}, nil) + + s, err := NewOpenShiftCAGetter(kubeClient).CABundle(context.Background()) + Expect(err).NotTo(HaveOccurred()) + Expect(s).To(BeEmpty()) + }) + + It("should work as expected", func() { + const value = "test-value" + + cm := v1.ConfigMap{ + Data: map[string]string{"ca-bundle.crt": value}, + } + + kubeClient. + EXPECT(). + GetConfigMap(context.Background(), "openshift-config", cmName, metav1.GetOptions{}). + Return(&cm, nil) + + s, err := NewOpenShiftCAGetter(kubeClient).CABundle(context.Background()) + Expect(err).NotTo(HaveOccurred()) + Expect(s).To( + Equal([]byte(value)), + ) + }) + }) +}) diff --git a/pkg/registry/wrapper.go b/pkg/registry/wrapper.go index 26220bb6b..2404a5337 100644 --- a/pkg/registry/wrapper.go +++ b/pkg/registry/wrapper.go @@ -18,6 +18,7 @@ import ( v1 "github.com/google/go-containerregistry/pkg/v1" "github.com/openshift/special-resource-operator/pkg/clients" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + ctrl "sigs.k8s.io/controller-runtime" ) const ( @@ -35,12 +36,14 @@ type CraneWrapper interface { type craneWrapper struct { kubeClient clients.ClientsInterface + ocg OpenShiftCAGetter sysCtx *types.SystemContext } -func NewCraneWrapper(kubeClient clients.ClientsInterface, registriesConfFilePath string) *craneWrapper { +func NewCraneWrapper(kubeClient clients.ClientsInterface, ocg OpenShiftCAGetter, registriesConfFilePath string) *craneWrapper { return &craneWrapper{ kubeClient: kubeClient, + ocg: ocg, sysCtx: &types.SystemContext{ SystemRegistriesConfPath: registriesConfFilePath, }, @@ -68,27 +71,40 @@ func (cw *craneWrapper) getPullSourcesForImageReference(imageName string) ([]sys } func (cw *craneWrapper) getClusterCustomCertPool(ctx context.Context) (*x509.CertPool, error) { + logger := ctrl.LoggerFrom(ctx) + + logger.Info("Loading system CA certificates") pool, err := x509.SystemCertPool() if err != nil { return nil, fmt.Errorf("could not access the system certificate pool: %w", err) } - img, err := cw.kubeClient.GetImage(ctx, imageClusterName, metav1.GetOptions{}) + logger.Info("Getting the bundle") + + caBundlePEM, err := cw.ocg.CABundle(ctx) if err != nil { - return nil, fmt.Errorf("could not retrieve image.config.openshift.io/%s: %w", imageClusterName, err) + return nil, fmt.Errorf("error getting the CA bundle: %v", err) } - cmName := img.Spec.AdditionalTrustedCA.Name - if cmName != "" { - cm, err := cw.kubeClient.GetConfigMap(ctx, configNamespace, cmName, metav1.GetOptions{}) - if err != nil { - return nil, fmt.Errorf("could not retrieve configmap %s/%s: %w", configNamespace, cmName, err) + if len(caBundlePEM) > 0 { + if ok := pool.AppendCertsFromPEM(caBundlePEM); !ok { + return nil, fmt.Errorf("could not append CA bundle to the pool: %v", err) } - for registry, cert := range cm.Data { - if ok := pool.AppendCertsFromPEM([]byte(cert)); !ok { - return nil, fmt.Errorf("could not append custom certificate for registry %s to the pool: %w", registry, err) - } + } + + logger.Info("Getting additional CAs") + + additionalCAs, err := cw.ocg.AdditionalTrustedCAs(ctx) + if err != nil { + return nil, fmt.Errorf("could not get additional trusted CAs: %v", err) + } + + for name, data := range additionalCAs { + logger.V(1).Info("Adding certificate", "name", name) + + if ok := pool.AppendCertsFromPEM(data); !ok { + return nil, fmt.Errorf("could not certificate %q to the pool: %v", name, err) } } diff --git a/pkg/registry/wrapper_test.go b/pkg/registry/wrapper_test.go index ae1a389cf..6ee771839 100644 --- a/pkg/registry/wrapper_test.go +++ b/pkg/registry/wrapper_test.go @@ -11,35 +11,35 @@ import ( v1 "k8s.io/api/core/v1" . "github.com/onsi/gomega" - configv1 "github.com/openshift/api/config/v1" "github.com/openshift/special-resource-operator/pkg/clients" ) var _ = Describe("Manifest", func() { const ( - registriesConfFile = "testdata/registries.conf" - expectedNamespace = "openshift-config" - expectedName = "pull-secret" - pullData = `{"auths":{"registry0.com":{"auth":"dXNlcm5hbWU6cGFzc3dvcmQK","email":"user@gmail.com"}}}` - img0 = "registry0.com/org/img" - img1 = "registry1.com/org/img" - registry0 = "registry0.com" - mirrorRegistry0 = "mirror-registry0.com" - additionalTrustedCAsCM = "disconnected-edge" - certFileName = "testdata/custom-ca-cert.pem" + registriesConfFile = "testdata/registries.conf" + expectedNamespace = "openshift-config" + expectedName = "pull-secret" + pullData = `{"auths":{"registry0.com":{"auth":"dXNlcm5hbWU6cGFzc3dvcmQK","email":"user@gmail.com"}}}` + img0 = "registry0.com/org/img" + img1 = "registry1.com/org/img" + registry0 = "registry0.com" + mirrorRegistry0 = "mirror-registry0.com" + certFileName = "testdata/custom-ca-cert.pem" ) var ( ctrl *gomock.Controller kubeClient *clients.MockClientsInterface + ocg *MockOpenShiftCAGetter cw CraneWrapper ) BeforeEach(func() { ctrl = gomock.NewController(GinkgoT()) kubeClient = clients.NewMockClientsInterface(ctrl) - cw = NewCraneWrapper(kubeClient, registriesConfFile) + ocg = NewMockOpenShiftCAGetter(ctrl) + cw = NewCraneWrapper(kubeClient, ocg, registriesConfFile) }) AfterEach(func() { @@ -83,7 +83,7 @@ var _ = Describe("Manifest", func() { }) It("should fail if registries.conf doesn't exist on the host", func() { - cw = NewCraneWrapper(kubeClient, "/non/existance/registries.conf") + cw = NewCraneWrapper(kubeClient, nil, "/non/existance/registries.conf") _, err := cw.(*craneWrapper).getPullSourcesForImageReference("registry0.com/org/img") Expect(err).To(HaveOccurred()) @@ -141,55 +141,76 @@ var _ = Describe("Manifest", func() { Context("getClusterCustomCertPool - dependency method", func() { - It("should fail if we can't access the image.config.openshift.io", func() { + It("should fail if CABundle returned an error", func() { + ocg.EXPECT().CABundle(context.Background()).Return(nil, errors.New("random error")) - kubeClient.EXPECT().GetImage(context.TODO(), imageClusterName, gomock.Any()).Return(nil, errors.New("some error")) + _, err := cw.(*craneWrapper).getClusterCustomCertPool(context.TODO()) + Expect(err).To(HaveOccurred()) + }) + + It("should fail if AdditionalTrustedCAs returned an error", func() { + gomock.InOrder( + ocg.EXPECT().CABundle(context.Background()), + ocg.EXPECT().AdditionalTrustedCAs(context.Background()).Return(nil, errors.New("random error")), + ) _, err := cw.(*craneWrapper).getClusterCustomCertPool(context.TODO()) Expect(err).To(HaveOccurred()) }) It("should add a certificate to the pool if the user had configured custom certificates in the cluster", func() { - - img := &configv1.Image{} - kubeClient.EXPECT().GetImage(context.TODO(), imageClusterName, gomock.Any()).Return(img, nil) + gomock.InOrder( + ocg.EXPECT().CABundle(context.Background()), + ocg.EXPECT().AdditionalTrustedCAs(context.Background()), + ) certPool, err := cw.(*craneWrapper).getClusterCustomCertPool(context.TODO()) Expect(err).NotTo(HaveOccurred()) + systemCertInPool := len(certPool.Subjects()) - img = &configv1.Image{ - Spec: configv1.ImageSpec{ - AdditionalTrustedCA: configv1.ConfigMapNameReference{ - Name: additionalTrustedCAsCM, - }, - }, - } data, err := os.ReadFile(certFileName) Expect(err).NotTo(HaveOccurred()) - cm := &v1.ConfigMap{ - Data: map[string]string{"some-registry": string(data)}, - } - kubeClient.EXPECT().GetImage(context.TODO(), imageClusterName, gomock.Any()).Return(img, nil) - kubeClient.EXPECT().GetConfigMap(context.TODO(), configNamespace, additionalTrustedCAsCM, gomock.Any()).Return(cm, nil) + + By("Having CABundle return a certificate") + + gomock.InOrder( + ocg.EXPECT().CABundle(context.Background()).Return(data, nil), + ocg.EXPECT().AdditionalTrustedCAs(context.Background()), + ) certPool, err = cw.(*craneWrapper).getClusterCustomCertPool(context.TODO()) Expect(err).NotTo(HaveOccurred()) - totalCertInPool := len(certPool.Subjects()) + Expect(certPool.Subjects()).To(HaveLen(systemCertInPool + 1)) + + By("Having AdditionalTrustedCAs return a certificate") + + gomock.InOrder( + ocg.EXPECT().CABundle(context.Background()), + ocg.EXPECT().AdditionalTrustedCAs(context.Background()).Return(map[string][]byte{"some-registry": data}, nil), + ) - Expect(totalCertInPool).To(Equal(systemCertInPool + 1)) + certPool, err = cw.(*craneWrapper).getClusterCustomCertPool(context.TODO()) + Expect(err).NotTo(HaveOccurred()) + Expect(certPool.Subjects()).To(HaveLen(systemCertInPool + 1)) }) }) It("is redirected to the mirrored image", func() { - img := &configv1.Image{} pullSecret := &v1.Secret{ Data: map[string][]byte{ ".dockerconfigjson": []byte(pullData), }, } - kubeClient.EXPECT().GetImage(context.TODO(), imageClusterName, gomock.Any()).Return(img, nil) - kubeClient.EXPECT().GetSecret(context.TODO(), configNamespace, pullSecretName, gomock.Any()).Return(pullSecret, nil) + + gomock.InOrder( + ocg.EXPECT().CABundle(context.Background()), + ocg.EXPECT().AdditionalTrustedCAs(context.Background()), + kubeClient. + EXPECT(). + GetSecret(context.TODO(), configNamespace, pullSecretName, gomock.Any()). + Return(pullSecret, nil), + ) _, err := cw.Manifest(context.TODO(), img0) // That registry doesn't exist so we will fail. We just want to make @@ -198,14 +219,20 @@ var _ = Describe("Manifest", func() { }) It("is accessing the correct image if no mirror was set", func() { - img := &configv1.Image{} pullSecret := &v1.Secret{ Data: map[string][]byte{ ".dockerconfigjson": []byte(pullData), }, } - kubeClient.EXPECT().GetImage(context.TODO(), imageClusterName, gomock.Any()).Return(img, nil) - kubeClient.EXPECT().GetSecret(context.TODO(), configNamespace, pullSecretName, gomock.Any()).Return(pullSecret, nil) + + gomock.InOrder( + ocg.EXPECT().CABundle(context.Background()), + ocg.EXPECT().AdditionalTrustedCAs(context.Background()), + kubeClient. + EXPECT(). + GetSecret(context.TODO(), configNamespace, pullSecretName, gomock.Any()). + Return(pullSecret, nil), + ) _, err := cw.Manifest(context.TODO(), img1) // That registry doesn't exist so we will fail. We just want to make