fix invalid endpoint in secret if no port provided

Author: Donatien26

internal/controller/user/reconcile.go

@@ -634,10 +634,11 @@ func (r *S3UserReconciler) handleCreate(
 			"s3region":        []byte(s3Client.GetConfig().Region),
 			"s3CACertificate": []byte(strings.Join(s3Client.GetConfig().CaCertificatesBase64, ",")),
 			"s3url":           []byte(s3Client.GetConfig().S3Url),
-			"s3ConnectionURL": []byte(fmt.Sprintf("https://%s:%s@%s",
+			"s3ConnectionURL": []byte(fmt.Sprintf("%s://%s:%s@%s",
+				s3Client.GetConfig().Scheme,
 				userResource.Spec.AccessKey,
 				secretKey,
-				strings.TrimPrefix(s3Client.GetConfig().Endpoint, "https://"),
+				s3Client.GetConfig().Endpoint,
 			),
 			),
 		})

internal/controller/user/reconcile_test.go

@@ -291,6 +291,7 @@ func TestHandleCreate(t *testing.T) {
 		assert.Equal(t, s3v1alpha1.Reconciled, s3UserResourceUpdated.Status.Conditions[0].Reason)
 		assert.Equal(t, metav1.ConditionTrue, s3UserResourceUpdated.Status.Conditions[0].Status)
 		assert.Contains(t, s3UserResourceUpdated.Status.Conditions[0].Message, "User reconciled")
+
 	})
 
 	t.Run("error if using invalidS3Instance", func(t *testing.T) {
@@ -310,6 +311,8 @@ func TestHandleCreate(t *testing.T) {
 		assert.Equal(t, "example-user", string(secretCreated.Data["accessKey"]))
 		assert.GreaterOrEqual(t, len(string(secretCreated.Data["secretKey"])), 20)
 		assert.NotEmpty(t, string(secretCreated.Data["s3ConnectionURL"]))
+		assert.Contains(t, string(secretCreated.Data["s3ConnectionURL"]), "@minio.example.com")
+		assert.Contains(t, string(secretCreated.Data["s3ConnectionURL"]), "https://example-user")
 	})
 }
 

internal/s3/client/impl/minioS3Client.go

@@ -125,19 +125,20 @@ func generateAdminMinioClient(
 	return minioAdminClient, nil
 }
 
-func ConstructEndpointFromURL(url string) (string, bool, error) {
+func ConstructEndpointFromURL(url string) (string, string, string, error) {
 	parsedURL, err := neturl.Parse(url)
 	if err != nil {
-		return "", false, fmt.Errorf("cannot detect if url use ssl or not")
+		return "", "", "", fmt.Errorf("cannot detect if url use ssl or not")
 	}
-
+	scheme := parsedURL.Scheme
 	endpoint := parsedURL.Hostname()
-	if (parsedURL.Scheme != "https" || parsedURL.Port() != "443") &&
-		(parsedURL.Scheme != "http" || parsedURL.Port() != "80") {
-		endpoint = fmt.Sprintf("%s:%s", endpoint, parsedURL.Port())
+	port := parsedURL.Port()
+	if port != "" && (scheme != "https" || port != "443") &&
+		(scheme != "http" || port != "80") {
+		endpoint = fmt.Sprintf("%s:%s", endpoint, port)
 	}
 
-	return endpoint, parsedURL.Scheme == "https", nil
+	return endpoint, port, scheme, nil
 }
 
 func addTlsClientConfigToMinioOptions(caCertificates []string, minioOptions *minio.Options) {

internal/s3/client/impl/minioS3Client_test.go

@@ -0,0 +1,118 @@
+/*
+Copyright 2023.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package s3clientimpl
+
+import (
+	"testing"
+)
+
+func TestConstructEndpointFromURL(t *testing.T) {
+	tests := []struct {
+		name     string
+		inputURL string
+		wantEP   string
+		wantPort string
+		wantSSL  bool
+		wantErr  bool
+	}{
+		{
+			name:     "https without port",
+			inputURL: "https://example.com",
+			wantEP:   "example.com",
+			wantPort: "",
+			wantSSL:  true,
+			wantErr:  false,
+		},
+		{
+			name:     "https with default port 443",
+			inputURL: "https://example.com:443",
+			wantEP:   "example.com",
+			wantPort: "443",
+			wantSSL:  true,
+			wantErr:  false,
+		},
+		{
+			name:     "http without port",
+			inputURL: "http://example.com",
+			wantEP:   "example.com",
+			wantPort: "",
+			wantSSL:  false,
+			wantErr:  false,
+		},
+		{
+			name:     "http with default port 80",
+			inputURL: "http://example.com:80",
+			wantEP:   "example.com",
+			wantPort: "80",
+			wantSSL:  false,
+			wantErr:  false,
+		},
+		{
+			name:     "https with non standard port",
+			inputURL: "https://example.com:8443",
+			wantEP:   "example.com:8443",
+			wantPort: "8443",
+			wantSSL:  true,
+			wantErr:  false,
+		},
+		{
+			name:     "http with non standard port",
+			inputURL: "http://example.com:8080",
+			wantEP:   "example.com:8080",
+			wantPort: "8080",
+			wantSSL:  false,
+			wantErr:  false,
+		},
+		{
+			name:     "invalid url",
+			inputURL: "://bad-url",
+			wantEP:   "",
+			wantPort: "",
+			wantSSL:  false,
+			wantErr:  true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			endpoint, port, scheme, err := ConstructEndpointFromURL(tt.inputURL)
+			ssl := scheme == "https"
+			if tt.wantErr {
+				if err == nil {
+					t.Fatalf("expected error, got nil")
+				}
+				return
+			}
+
+			if err != nil {
+				t.Fatalf("unexpected error: %v", err)
+			}
+
+			if endpoint != tt.wantEP {
+				t.Errorf("endpoint = %q, want %q", endpoint, tt.wantEP)
+			}
+
+			if port != tt.wantPort {
+				t.Errorf("port = %q, want %q", port, tt.wantPort)
+			}
+
+			if ssl != tt.wantSSL {
+				t.Errorf("ssl = %v, want %v", ssl, tt.wantSSL)
+			}
+		})
+	}
+}

internal/s3/client/s3client.go

@@ -34,6 +34,8 @@ type S3Config struct {
 	PolicyDeletionEnabled bool
 	Secure                bool
 	Endpoint              string
+	Port                  string
+	Scheme                string
 	PathStyle             bool
 }
 

internal/s3/factory/impl/s3factoryImpl.go

@@ -32,14 +32,16 @@ func NewS3Factory() *S3Factory {
 
 func (mockedS3Provider *S3Factory) GenerateS3Client(s3Provider string, s3Config *s3client.S3Config) (s3client.S3Client, error) {
 	s3Logger := ctrl.Log.WithValues("logger", "s3factory")
-	endpoint, isSSL, err := s3clientImpl.ConstructEndpointFromURL(s3Config.S3Url)
+	endpoint, port, scheme, err := s3clientImpl.ConstructEndpointFromURL(s3Config.S3Url)
 	if err != nil {
 		s3Logger.Error(err, "an error occurred while creating a new minio client")
 		return nil, err
 	}
-	s3Config.Secure = isSSL
+	s3Config.Secure = scheme == "https"
 	s3Config.Endpoint = endpoint
+	s3Config.Scheme = scheme
 	s3Config.PathStyle = false
+	s3Config.Port = port
 	if s3Provider == "mockedS3Provider" {
 		return s3clientImpl.NewMockedS3Client(s3Config), nil
 	}