Apply review's input

pkg/controller/elasticsearch/client/client.go

@@ -164,6 +164,11 @@ func (e *APIError) Error() string {
 	return fmt.Sprintf("%s: %s", e.response.Status, reason)
 }
 
+// IsUnauthorized checks whether the error was an HTTP 401 error.
+func IsUnauthorized(err error) bool {
+	return isHTTPError(err, http.StatusUnauthorized)
+}
+
 // IsForbidden checks whether the error was an HTTP 403 error.
 func IsForbidden(err error) bool {
 	return isHTTPError(err, http.StatusForbidden)

pkg/controller/elasticsearch/driver/driver.go

@@ -198,10 +198,10 @@ func (d *defaultDriver) Reconcile(ctx context.Context) *reconciler.Results {
 
 	if esReachable { //nolint:nestif
 		// reconcile the Elasticsearch license
-		unsupportedElasticsearch, err := license.Reconcile(ctx, d.Client, d.ES, esClient)
+		supportedDistribution, err := license.Reconcile(ctx, d.Client, d.ES, esClient)
 		if err != nil {
-			if unsupportedElasticsearch {
-				msg := "Unsupported Elasticsearch"
+			if !supportedDistribution {
+				msg := "Unsupported Elasticsearch distribution"
 				d.ReconcileState.AddEvent(corev1.EventTypeWarning, events.EventReasonUnexpected, fmt.Sprintf("%s: %s", msg, err.Error()))
 				log.Error(err, msg, "namespace", d.ES.Namespace, "es_name", d.ES.Name)
 				// unsupported Elasticsearch, let's update the phase to "invalid" and stop the reconciliation

pkg/controller/elasticsearch/license/apply_test.go

@@ -235,47 +235,59 @@ func Test_applyLinkedLicense(t *testing.T) {
 
 func Test_checkEsLicense(t *testing.T) {
 	tests := []struct {
-		name        string
-		wantErr     bool
-		unsupported bool
-		updater     esclient.LicenseClient
+		name      string
+		wantErr   bool
+		supported bool
+		updater   esclient.LicenseClient
 	}{
 		{
-			name:        "happy path",
-			wantErr:     false,
-			unsupported: false,
+			name:      "happy path",
+			wantErr:   false,
+			supported: true,
 			updater: &fakeInvalidLicenseUpdater{
 				fakeLicenseUpdater:     &fakeLicenseUpdater{license: esclient.License{Type: string(esclient.ElasticsearchLicenseTypeBasic)}},
 				statusCodeOnGetLicense: 200,
 			},
 		},
 		{
-			name:        "error: 400 on get license, unsupported",
-			wantErr:     true,
-			unsupported: true,
-			updater:     &fakeInvalidLicenseUpdater{statusCodeOnGetLicense: 400},
+			name:      "error: 400 on get license, unsupported distribution",
+			wantErr:   true,
+			supported: false,
+			updater:   &fakeInvalidLicenseUpdater{statusCodeOnGetLicense: 400},
 		},
 		{
-			name:        "error: 404 on get license, supported",
-			wantErr:     true,
-			unsupported: false,
-			updater:     &fakeInvalidLicenseUpdater{statusCodeOnGetLicense: 404},
+			name:      "error: 401 on get license",
+			wantErr:   true,
+			supported: true,
+			updater:   &fakeInvalidLicenseUpdater{statusCodeOnGetLicense: 401},
 		},
 		{
-			name:        "error: 500 on get license, supported",
-			wantErr:     true,
-			unsupported: false,
-			updater:     &fakeInvalidLicenseUpdater{statusCodeOnGetLicense: 500},
+			name:      "error: 403 on get license",
+			wantErr:   true,
+			supported: true,
+			updater:   &fakeInvalidLicenseUpdater{statusCodeOnGetLicense: 403},
+		},
+		{
+			name:      "error: 404 on get license",
+			wantErr:   true,
+			supported: true,
+			updater:   &fakeInvalidLicenseUpdater{statusCodeOnGetLicense: 404},
+		},
+		{
+			name:      "error: 500 on get license",
+			wantErr:   true,
+			supported: true,
+			updater:   &fakeInvalidLicenseUpdater{statusCodeOnGetLicense: 500},
 		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			_, unsupported, err := checkEsLicense(context.Background(), tt.updater)
+			_, supported, err := checkElasticsearchLicense(context.Background(), tt.updater)
 			if (err != nil) != tt.wantErr {
-				t.Errorf("checkEsLicense() error = %v, wantErr %v", err, tt.wantErr)
+				t.Errorf("checkElasticsearchLicense() error = %v, wantErr %v", err, tt.wantErr)
 			}
-			if unsupported != tt.unsupported {
-				t.Errorf("checkEsLicense() unsupported = %v, unsupported %v", unsupported, tt.unsupported)
+			if supported != tt.supported {
+				t.Errorf("checkElasticsearchLicense() supported = %v, supported %v", supported, tt.supported)
 			}
 		})
 	}

pkg/controller/elasticsearch/license/reconcile.go

@@ -10,6 +10,7 @@ import (
 	esv1 "github.com/elastic/cloud-on-k8s/pkg/apis/elasticsearch/v1"
 	esclient "github.com/elastic/cloud-on-k8s/pkg/controller/elasticsearch/client"
 	"github.com/elastic/cloud-on-k8s/pkg/utils/k8s"
+	"github.com/pkg/errors"
 )
 
 // Reconcile reconciles the current Elasticsearch license with the desired one.
@@ -19,24 +20,31 @@ func Reconcile(
 	esCluster esv1.Elasticsearch,
 	clusterClient esclient.Client,
 ) (bool, error) {
-	currentLicense, unsupportedElasticsearch, err := checkElasticsearchLicense(ctx, clusterClient)
+	currentLicense, supportedDistribution, err := checkElasticsearchLicense(ctx, clusterClient)
 	if err != nil {
-		return unsupportedElasticsearch, err
+		return supportedDistribution, err
 	}
 
 	clusterName := k8s.ExtractNamespacedName(&esCluster)
 	return true, applyLinkedLicense(ctx, c, clusterName, clusterClient, currentLicense)
 }
 
 // checkElasticsearchLicense checks that Elasticsearch is licensed, which ensures that the operator is communicating
-// with a supported version of Elasticsearch
+// with a supported Elasticsearch distribution
 func checkElasticsearchLicense(ctx context.Context, clusterClient esclient.LicenseClient) (esclient.License, bool, error) {
+	supportedDistribution := true
 	currentLicense, err := clusterClient.GetLicense(ctx)
 	if err != nil {
-		// 4xx is not supported, except 404 which may happen if the master node is generating a new cluster state
-		unsupportedElasticsearch := esclient.Is4xx(err) && !esclient.IsNotFound(err)
-		return esclient.License{}, unsupportedElasticsearch, err
+		if esclient.IsUnauthorized(err) {
+			err = errors.New("unauthorized access, unable to verify Elasticsearch license, check your security configuration")
+		} else if esclient.IsForbidden(err) {
+			err = errors.New("forbidden access, unable to verify Elasticsearch license, check your security configuration")
+		} else if esclient.IsNotFound(err) {
+			// 404 may happen if the master node is generating a new cluster state
+		} else if esclient.Is4xx(err) {
+			supportedDistribution = false
+			err = errors.Wrap(err, "unable to verify Elasticsearch license")
+		}
 	}
-
-	return currentLicense, false, nil
+	return currentLicense, supportedDistribution, err
 }