diff --git a/.golangci.yaml b/.golangci.yaml index 937a37eac..79e2af08e 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -7,5 +7,8 @@ linters: - ginkgolinter - gofmt - govet + - gosec + - errname + - err113 run: timeout: 5m diff --git a/api/v1beta1/conditions.go b/api/v1beta1/conditions.go index b8f4cf239..c7bb9e214 100644 --- a/api/v1beta1/conditions.go +++ b/api/v1beta1/conditions.go @@ -56,6 +56,9 @@ const ( // Common Messages used by API objects. const ( + //NovaNetworkAttachmentsReadyInitMessage + NovaNetworkAttachmentsReadyErrorMessage = "NetworkAttachments error occurred not all pods have interfaces with ips as configured in NetworkAttachments: %s" + // NovaAPIReadyInitMessage NovaAPIReadyInitMessage = "NovaAPI not started" @@ -172,4 +175,7 @@ const ( //CellHostDiscoverErrorMessage CellHostDiscoverErrorMessage = "CellHostDiscover error occurred %s" + + //InputReadyErrorMessage + InputReadyErrorMessage = "Input data error occurred %s" ) diff --git a/controllers/common.go b/controllers/common.go index a37484c38..66105bfd2 100644 --- a/controllers/common.go +++ b/controllers/common.go @@ -95,7 +95,7 @@ const ( // fields to index to reconcile when change passwordSecretField = ".spec.secret" - caBundleSecretNameField = ".spec.tls.caBundleSecretName" + caBundleSecretNameField = ".spec.tls.caBundleSecretName" // #nosec G101 tlsAPIInternalField = ".spec.tls.api.internal.secretName" tlsAPIPublicField = ".spec.tls.api.public.secretName" tlsMetadataField = ".spec.tls.secretName" @@ -186,8 +186,6 @@ type conditionUpdater interface { MarkTrue(t condition.Type, messageFormat string, messageArgs ...interface{}) } -// ensureSecret - ensures that the Secret object exists and the expected fields -// are in the Secret. It returns a hash of the values of the expected fields. func ensureSecret( ctx context.Context, secretName types.NamespacedName, @@ -225,7 +223,10 @@ func ensureSecret( for _, field := range expectedFields { val, ok := secret.Data[field] if !ok { - err := fmt.Errorf("field '%s' not found in secret/%s", field, secretName.Name) + // Usage in the code + err := k8s_errors.NewBadRequest(fmt.Sprintf("field '%s' not found in secret/%s", field, secretName.Name)) + // ensureSecret - ensures that the Secret object exists and the expected fields + // are in the Secret. It returns a hash of the values of the expected fields. conditionUpdater.Set(condition.FalseCondition( condition.InputReadyCondition, condition.ErrorReason, @@ -579,7 +580,7 @@ func getNovaClient( return nil, err } if (ctrlResult != ctrl.Result{}) { - return nil, fmt.Errorf("the CABundleSecret %s not found", auth.GetCABundleSecretName()) + return nil, fmt.Errorf("the CABundleSecret %s not found: %w", auth.GetCABundleSecretName(), err) } tlsConfig = &openstack.TLSConfig{ @@ -641,7 +642,7 @@ func ensureMemcached( condition.RequestedReason, condition.SeverityInfo, condition.MemcachedReadyWaitingMessage)) - return nil, fmt.Errorf("memcached %s not found", memcachedName) + return nil, fmt.Errorf("%w: memcached %s not found", err, memcachedName) } conditionUpdater.Set(condition.FalseCondition( condition.MemcachedReadyCondition, @@ -658,7 +659,7 @@ func ensureMemcached( condition.RequestedReason, condition.SeverityInfo, condition.MemcachedReadyWaitingMessage)) - return nil, fmt.Errorf("memcached %s is not ready", memcachedName) + return nil, fmt.Errorf("%w: memcached %s is not ready", ErrResourceIsNotReady, memcachedName) } conditionUpdater.MarkTrue(condition.MemcachedReadyCondition, condition.MemcachedReadyMessage) diff --git a/controllers/errors.go b/controllers/errors.go new file mode 100644 index 000000000..7f4c64842 --- /dev/null +++ b/controllers/errors.go @@ -0,0 +1,12 @@ +package controllers + +import ( + "errors" +) + +var ( + ErrResourceIsNotReady = errors.New("Resource is not ready") + ErrInvalidStatus = errors.New("invalid status") + ErrCannotUpdateObject = errors.New("cannot update object") + ErrFieldNotFound = errors.New("field not found in Secret") +) diff --git a/controllers/nova_controller.go b/controllers/nova_controller.go index ac91fa849..cb18abe74 100644 --- a/controllers/nova_controller.go +++ b/controllers/nova_controller.go @@ -302,7 +302,7 @@ func (r *NovaReconciler) Reconcile(ctx context.Context, req ctrl.Request) (resul case nova.DBCompleted: instance.Status.Conditions.MarkTrue(novav1.NovaAPIDBReadyCondition, condition.DBReadyMessage) default: - return ctrl.Result{}, fmt.Errorf("invalid DatabaseStatus from ensureAPIDB: %d", apiDBStatus) + return ctrl.Result{}, fmt.Errorf("%w from ensureAPIDB: %d", ErrInvalidStatus, apiDBStatus) } // We need to create a list of cellNames to iterate on and as the map @@ -332,7 +332,7 @@ func (r *NovaReconciler) Reconcile(ctx context.Context, req ctrl.Request) (resul creatingDBs = append(creatingDBs, cellName) case nova.DBCompleted: default: - return ctrl.Result{}, fmt.Errorf("invalid DatabaseStatus from ensureCellDB: %d for cell %s", status, cellName) + return ctrl.Result{}, fmt.Errorf("%w from ensureCellDB: %d for cell %s", ErrInvalidStatus, status, cellName) } cellDBs[cellName] = &nova.Database{Database: cellDB, Status: status} } @@ -380,7 +380,7 @@ func (r *NovaReconciler) Reconcile(ctx context.Context, req ctrl.Request) (resul instance.Status.Conditions.MarkTrue( novav1.NovaAPIMQReadyCondition, novav1.NovaAPIMQReadyMessage) default: - return ctrl.Result{}, fmt.Errorf("invalid MessageBusStatus from for the API MQ: %d", apiMQStatus) + return ctrl.Result{}, fmt.Errorf("%w from for the API MQ: %d", ErrInvalidStatus, apiMQStatus) } cellMQs := map[string]*nova.MessageBus{} @@ -408,7 +408,7 @@ func (r *NovaReconciler) Reconcile(ctx context.Context, req ctrl.Request) (resul creatingMQs = append(creatingMQs, cellName) case nova.MQCompleted: default: - return ctrl.Result{}, fmt.Errorf("invalid MessageBusStatus from ensureMQ: %d for cell %s", status, cellName) + return ctrl.Result{}, fmt.Errorf("%w from ensureMQ: %d for cell %s", ErrInvalidStatus, status, cellName) } cellMQs[cellName] = &nova.MessageBus{TransportURL: cellTransportURL, Status: status} } @@ -1631,8 +1631,8 @@ func (r *NovaReconciler) ensureMQ( url, ok := secret.Data[TransportURLSelector] if !ok { - return "", nova.MQFailed, fmt.Errorf( - "the TransportURL secret %s does not have 'transport_url' field", transportURL.Status.SecretName) + return "", nova.MQFailed, k8s_errors.NewBadRequest(fmt.Sprintf( + "the TransportURL secret %s does not have 'transport_url' field", transportURL.Status.SecretName)) } return string(url), nova.MQCompleted, nil @@ -1673,7 +1673,7 @@ func (r *NovaReconciler) ensureMetadata( // If it is not created by us, we don't touch it if !k8s_errors.IsNotFound(err) && !OwnedBy(metadata, instance) { err := fmt.Errorf( - "cannot update NovaMetadata/%s as the cell is not owning it", metadata.Name) + "%w NovaMetadata/%s as the cell is not owning it", ErrCannotUpdateObject, metadata.Name) Log.Error(err, "NovaMetadata is enabled, but there is a "+ "NovaMetadata CR not owned by us. We cannot update it. "+ diff --git a/controllers/novaapi_controller.go b/controllers/novaapi_controller.go index 884c725d5..45d4af9c4 100644 --- a/controllers/novaapi_controller.go +++ b/controllers/novaapi_controller.go @@ -588,13 +588,12 @@ func (r *NovaAPIReconciler) ensureDeployment( if networkReady { instance.Status.Conditions.MarkTrue(condition.NetworkAttachmentsReadyCondition, condition.NetworkAttachmentsReadyMessage) } else { - err := fmt.Errorf("not all pods have interfaces with ips as configured in NetworkAttachments: %s", instance.Spec.NetworkAttachments) instance.Status.Conditions.Set(condition.FalseCondition( condition.NetworkAttachmentsReadyCondition, condition.ErrorReason, condition.SeverityWarning, - condition.NetworkAttachmentsReadyErrorMessage, - err.Error())) + novav1.NovaNetworkAttachmentsReadyErrorMessage, + instance.Spec.NetworkAttachments)) return ctrl.Result{}, err } diff --git a/controllers/novacell_controller.go b/controllers/novacell_controller.go index 1a447912b..d7f4ba80c 100644 --- a/controllers/novacell_controller.go +++ b/controllers/novacell_controller.go @@ -426,7 +426,7 @@ func (r *NovaCellReconciler) ensureNoVNCProxy( // If it is not created by us, we don't touch it if !k8s_errors.IsNotFound(err) && !OwnedBy(novncproxy, instance) { err := fmt.Errorf( - "cannot update NovaNoVNCProxy/%s as the cell is not owning it", novncproxyName.Name) + "%w NovaNoVNCProxy/%s as the cell is not owning it", ErrCannotUpdateObject, novncproxyName.Name) Log.Error(err, "NovaNoVNCProxy is enabled in this cell, but there is a "+ "NovaNoVNCProxy CR not owned by the cell. We cannot update it. "+ @@ -548,7 +548,7 @@ func (r *NovaCellReconciler) ensureMetadata( // If it is not created by us, we don't touch it if !k8s_errors.IsNotFound(err) && !OwnedBy(metadata, instance) { err := fmt.Errorf( - "cannot update NovaMetadata/%s as the cell is not owning it", metadata.Name) + "%w NovaMetadata/%s as the cell is not owning it", ErrCannotUpdateObject, metadata.Name) Log.Error(err, "NovaMetadata is enabled, but there is a "+ "NovaMetadata CR not owned by us. We cannot update it. "+ @@ -708,7 +708,7 @@ func (r *NovaCellReconciler) ensureNovaCompute( // If it is not created by us, we don't touch it if !k8s_errors.IsNotFound(err) && !OwnedBy(novacompute, instance) { err := fmt.Errorf( - "cannot update NovaCompute/%s as the cell is not owning it", novacompute.Name) + "%w NovaCompute/%s as the cell is not owning it", ErrCannotUpdateObject, novacompute.Name) Log.Error(err, "NovaCompute is defined in the cell, but there is a "+ "NovaCompute CR not owned by us. We cannot update it. "+ diff --git a/controllers/novacompute_controller.go b/controllers/novacompute_controller.go index 663bfda6f..229adcfc5 100644 --- a/controllers/novacompute_controller.go +++ b/controllers/novacompute_controller.go @@ -403,13 +403,12 @@ func (r *NovaComputeReconciler) ensureDeployment( if networkReady { instance.Status.Conditions.MarkTrue(condition.NetworkAttachmentsReadyCondition, condition.NetworkAttachmentsReadyMessage) } else { - err := fmt.Errorf("not all pods have interfaces with ips as configured in NetworkAttachments: %s", instance.Spec.NetworkAttachments) instance.Status.Conditions.Set(condition.FalseCondition( condition.NetworkAttachmentsReadyCondition, condition.ErrorReason, condition.SeverityWarning, - condition.NetworkAttachmentsReadyErrorMessage, - err.Error())) + novav1.NovaNetworkAttachmentsReadyErrorMessage, + instance.Spec.NetworkAttachments)) return ctrl.Result{}, err } diff --git a/controllers/novaconductor_controller.go b/controllers/novaconductor_controller.go index cb3944294..72d26bfc0 100644 --- a/controllers/novaconductor_controller.go +++ b/controllers/novaconductor_controller.go @@ -560,13 +560,12 @@ func (r *NovaConductorReconciler) ensureDeployment( if networkReady { instance.Status.Conditions.MarkTrue(condition.NetworkAttachmentsReadyCondition, condition.NetworkAttachmentsReadyMessage) } else { - err := fmt.Errorf("not all pods have interfaces with ips as configured in NetworkAttachments: %s", instance.Spec.NetworkAttachments) instance.Status.Conditions.Set(condition.FalseCondition( condition.NetworkAttachmentsReadyCondition, condition.ErrorReason, condition.SeverityWarning, - condition.NetworkAttachmentsReadyErrorMessage, - err.Error())) + novav1.NovaNetworkAttachmentsReadyErrorMessage, + instance.Spec.NetworkAttachments)) return ctrl.Result{}, err } diff --git a/controllers/novametadata_controller.go b/controllers/novametadata_controller.go index a1f0c9945..b53d534b7 100644 --- a/controllers/novametadata_controller.go +++ b/controllers/novametadata_controller.go @@ -598,13 +598,12 @@ func (r *NovaMetadataReconciler) ensureDeployment( if networkReady { instance.Status.Conditions.MarkTrue(condition.NetworkAttachmentsReadyCondition, condition.NetworkAttachmentsReadyMessage) } else { - err := fmt.Errorf("not all pods have interfaces with ips as configured in NetworkAttachments: %s", instance.Spec.NetworkAttachments) instance.Status.Conditions.Set(condition.FalseCondition( condition.NetworkAttachmentsReadyCondition, condition.ErrorReason, condition.SeverityWarning, - condition.NetworkAttachmentsReadyErrorMessage, - err.Error())) + novav1.NovaNetworkAttachmentsReadyErrorMessage, + instance.Spec.NetworkAttachments)) return ctrl.Result{}, err } diff --git a/controllers/novanovncproxy_controller.go b/controllers/novanovncproxy_controller.go index f0f0c0962..1ef897497 100644 --- a/controllers/novanovncproxy_controller.go +++ b/controllers/novanovncproxy_controller.go @@ -546,13 +546,12 @@ func (r *NovaNoVNCProxyReconciler) ensureDeployment( if networkReady { instance.Status.Conditions.MarkTrue(condition.NetworkAttachmentsReadyCondition, condition.NetworkAttachmentsReadyMessage) } else { - err := fmt.Errorf("not all pods have interfaces with ips as configured in NetworkAttachments: %s", instance.Spec.NetworkAttachments) instance.Status.Conditions.Set(condition.FalseCondition( condition.NetworkAttachmentsReadyCondition, condition.ErrorReason, condition.SeverityWarning, - condition.NetworkAttachmentsReadyErrorMessage, - err.Error())) + novav1.NovaNetworkAttachmentsReadyErrorMessage, + instance.Spec.NetworkAttachments)) return ctrl.Result{}, err } diff --git a/controllers/novascheduler_controller.go b/controllers/novascheduler_controller.go index 941915abc..1571cdf62 100644 --- a/controllers/novascheduler_controller.go +++ b/controllers/novascheduler_controller.go @@ -613,13 +613,12 @@ func (r *NovaSchedulerReconciler) ensureDeployment( if networkReady { instance.Status.Conditions.MarkTrue(condition.NetworkAttachmentsReadyCondition, condition.NetworkAttachmentsReadyMessage) } else { - err := fmt.Errorf("not all pods have interfaces with ips as configured in NetworkAttachments: %s", instance.Spec.NetworkAttachments) instance.Status.Conditions.Set(condition.FalseCondition( condition.NetworkAttachmentsReadyCondition, condition.ErrorReason, condition.SeverityWarning, - condition.NetworkAttachmentsReadyErrorMessage, - err.Error())) + novav1.NovaNetworkAttachmentsReadyErrorMessage, + instance.Spec.NetworkAttachments)) return ctrl.Result{}, err } diff --git a/test/functional/novacell_controller_test.go b/test/functional/novacell_controller_test.go index fd1ddbb75..0643889c8 100644 --- a/test/functional/novacell_controller_test.go +++ b/test/functional/novacell_controller_test.go @@ -696,7 +696,7 @@ var _ = Describe("NovaCell controller", func() { corev1.ConditionFalse, condition.ErrorReason, fmt.Sprintf( - "NovaNoVNCProxy error occurred cannot update "+ + "NovaNoVNCProxy error occurred cannot update object "+ "NovaNoVNCProxy/%s as the cell is not owning it", cell2.NoVNCProxyName.Name, ), @@ -833,7 +833,7 @@ var _ = Describe("NovaCell controller", func() { corev1.ConditionFalse, condition.ErrorReason, fmt.Sprintf( - "NovaMetadata error occurred cannot update "+ + "NovaMetadata error occurred cannot update object "+ "NovaMetadata/%s as the cell is not owning it", cell2.MetadataName.Name, ), diff --git a/test/functional/suite_test.go b/test/functional/suite_test.go index 7a30ea003..6abcb5712 100644 --- a/test/functional/suite_test.go +++ b/test/functional/suite_test.go @@ -244,7 +244,7 @@ var _ = BeforeSuite(func() { dialer := &net.Dialer{Timeout: time.Duration(10) * time.Second} addrPort := fmt.Sprintf("%s:%d", webhookInstallOptions.LocalServingHost, webhookInstallOptions.LocalServingPort) Eventually(func() error { - conn, err := tls.DialWithDialer(dialer, "tcp", addrPort, &tls.Config{InsecureSkipVerify: true}) + conn, err := tls.DialWithDialer(dialer, "tcp", addrPort, &tls.Config{InsecureSkipVerify: true}) // #nosec G402 if err != nil { return err }