From 82f693a19a6b9d6d9f3c4a1e413ab21fe7d53ee3 Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Thu, 17 Apr 2025 10:29:57 +0100 Subject: [PATCH 1/2] trivial: Use errors.New rather than fmt.Errorf Where fmt.Errorf is called with a single string parameter, it is faster and arguably more correct to call errors.New instead. Signed-off-by: Stephen Finucane --- pkg/operator/secretannotator/openstack/reconciler.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/pkg/operator/secretannotator/openstack/reconciler.go b/pkg/operator/secretannotator/openstack/reconciler.go index 9cb3d70e0f..eb8a5e2eaa 100644 --- a/pkg/operator/secretannotator/openstack/reconciler.go +++ b/pkg/operator/secretannotator/openstack/reconciler.go @@ -18,6 +18,7 @@ package openstack import ( "context" + "errors" "fmt" "time" @@ -134,7 +135,7 @@ func (r *ReconcileCloudCredSecret) Reconcile(ctx context.Context, request reconc } if conflict { r.Logger.Error("configuration conflict between legacy configmap and operator config") - return reconcile.Result{}, fmt.Errorf("configuration conflict") + return reconcile.Result{}, errors.New("configuration conflict") } if mode == operatorv1.CloudCredentialsModeManual { r.Logger.Info("operator in disabled / manual mode") @@ -146,7 +147,7 @@ func (r *ReconcileCloudCredSecret) Reconcile(ctx context.Context, request reconc default: const msg = "OpenStack only supports Passthrough mode" r.Logger.Error(msg) - return reconcile.Result{}, fmt.Errorf(msg) + return reconcile.Result{}, errors.New(msg) } secret := &corev1.Secret{} From 852f4f6f1e044f013b1918d7b5024e0a8b04bd19 Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Thu, 17 Apr 2025 10:27:00 +0100 Subject: [PATCH 2/2] openstack: Sync cacert from CCM to root credential There's some rework needed around CCM and the docs to get users to start using the new location of the CA cert. That is not going to happen in 4.19, so for now we opt to simply sync from the old place to the new place and leave the existing docs in place. In a future release, we can fully remove the old place (with a release note) and remove this syncer. Signed-off-by: Stephen Finucane --- .../secretannotator/openstack/reconciler.go | 19 ++++++++++++++++++- .../openstack/reconciler_test.go | 12 ++++++++++-- 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/pkg/operator/secretannotator/openstack/reconciler.go b/pkg/operator/secretannotator/openstack/reconciler.go index eb8a5e2eaa..e677a13278 100644 --- a/pkg/operator/secretannotator/openstack/reconciler.go +++ b/pkg/operator/secretannotator/openstack/reconciler.go @@ -163,13 +163,30 @@ func (r *ReconcileCloudCredSecret) Reconcile(ctx context.Context, request reconc return reconcile.Result{}, err } + // Sync the cacert from its legacy location (the 'ca-bundle.pem' key of the + // 'openshift-config / cloud-provider-config' CM) to the new place, if present. + // TODO(stephenfin): Remove this syncer in a future release once CCM no longer + // relies on the legacy place during bootstrapping. + config := &corev1.ConfigMap{} + err = r.RootCredClient.Get(context.Background(), types.NamespacedName{Namespace: "openshift-config", Name: "cloud-provider-config"}, config) + if err != nil { + r.Logger.Debugf("cloud provider config not found: %v", err) + return reconcile.Result{}, err + } + + cacertUpdated := false + if ccmCACert := config.Data["ca-bundle.pem"]; ccmCACert != cacert { + cacert = ccmCACert + cacertUpdated = true + } + clouds, cloudsUpdated, err := r.fixInvalidCACertFile(clouds) if err != nil { r.Logger.WithError(err).Error("errored checking clouds.yaml") return reconcile.Result{}, err } - if cloudsUpdated { + if cloudsUpdated || cacertUpdated { openstack.SetRootCloudCredentialsSecretData(secret, clouds, cacert) err := r.RootCredClient.Update(context.TODO(), secret) if err != nil { diff --git a/pkg/operator/secretannotator/openstack/reconciler_test.go b/pkg/operator/secretannotator/openstack/reconciler_test.go index e738918037..32aab010c0 100644 --- a/pkg/operator/secretannotator/openstack/reconciler_test.go +++ b/pkg/operator/secretannotator/openstack/reconciler_test.go @@ -108,6 +108,14 @@ func TestReconcileCloudCredSecret_Reconcile(t *testing.T) { }, } + ccmConfig := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cloud-provider-config", + Namespace: "openshift-config", + }, + Data: map[string]string{}, + } + /* Test parsing of CCO configuration and the resulting annotation of the root secret. Most of this is boilerplate behaviour. @@ -181,7 +189,7 @@ func TestReconcileCloudCredSecret_Reconcile(t *testing.T) { secret := testSecret(fmt.Sprintf(cloudsWithCACert, correctCACertFile)) existing := append(tc.existing, infra, testOperatorConfig(tc.mode)) fakeClient := fake.NewClientBuilder().WithRuntimeObjects(existing...).Build() - fakeRootCredClient := fake.NewClientBuilder().WithRuntimeObjects(secret).Build() + fakeRootCredClient := fake.NewClientBuilder().WithRuntimeObjects(secret, ccmConfig).Build() r := &ReconcileCloudCredSecret{ Client: fakeClient, @@ -270,7 +278,7 @@ func TestReconcileCloudCredSecret_Reconcile(t *testing.T) { t.Run(tc.name, func(t *testing.T) { secret := testSecret(tc.cloudsYAML) fakeClient := fake.NewClientBuilder().WithRuntimeObjects(infra, passthrough).Build() - fakeRootCredClient := fake.NewClientBuilder().WithRuntimeObjects(secret).Build() + fakeRootCredClient := fake.NewClientBuilder().WithRuntimeObjects(secret, ccmConfig).Build() t.Logf("clouds.yaml: %s", tc.cloudsYAML) r := &ReconcileCloudCredSecret{