Fix minio ssl compatible issue

Signed-off-by: yhmo <[email protected]>
yhmo · Mar 26, 2024 · 550ab26 · 550ab26
1 parent 73858b2
commit 550ab26
Show file tree

Hide file tree

Showing 6 changed files with 68 additions and 18 deletions.
diff --git a/configs/milvus.yaml b/configs/milvus.yaml
@@ -68,8 +68,8 @@ minio:
   port: 9000 # Port of MinIO/S3
   accessKeyID: minioadmin # accessKeyID of MinIO/S3
   secretAccessKey: minioadmin # MinIO/S3 encryption string
+  useSSL: false # Access to MinIO/S3 with SSL
   ssl:
-    enabled: false # Access to MinIO/S3 with SSL
     tlsCACert: /path/to/public.crt # path to your CACert file, ignore when it is empty
   bucketName: a-bucket # Bucket name in MinIO/S3
   rootPath: files # The root path where the message is stored in MinIO/S3

diff --git a/internal/core/src/storage/ChunkManager.cpp b/internal/core/src/storage/ChunkManager.cpp
@@ -53,16 +53,20 @@ generateConfig(const StorageConfig& storage_config) {
     Aws::Client::ClientConfiguration config = g_config;
     config.endpointOverride = ConvertToAwsString(storage_config.address);
 
+    // Three cases:
+    // 1. If the MinIO server SSL is disabled, set config.verifySSL = false
+    // 2. If the MinIO server is self-signed certificate, TLS certificate verification is skipped, set config.verifySSL = false
+    // 3. If the MinIO server uses a CA-signed certificate, set config.verifySSL = true.
+    config.verifySSL = false;
     if (storage_config.useSSL) {
         config.scheme = Aws::Http::Scheme::HTTPS;
+        if (!storage_config.sslCACert.empty()) {
+            config.caPath = ConvertToAwsString(storage_config.sslCACert);
+            config.verifySSL = true;  // uses a CA-signed certificate
+        }
     } else {
-        config.scheme = Aws::Http::Scheme::HTTP;
-    }
-
-    if (!storage_config.sslCACert.empty()) {
-        config.caPath = ConvertToAwsString(storage_config.sslCACert);
+        config.scheme = Aws::Http::Scheme::HTTP; // SSL is disabled
     }
-    config.verifySSL = false;
 
     if (!storage_config.region.empty()) {
         config.region = ConvertToAwsString(storage_config.region);

diff --git a/internal/core/src/storage/MinioChunkManager.cpp b/internal/core/src/storage/MinioChunkManager.cpp
@@ -322,17 +322,21 @@ MinioChunkManager::MinioChunkManager(const StorageConfig& storage_config)
     Aws::Client::ClientConfiguration config = g_config;
     config.endpointOverride = ConvertToAwsString(storage_config.address);
 
+    // Three cases:
+    // 1. If the MinIO server SSL is disabled, set config.verifySSL = false
+    // 2. If the MinIO server is self-signed certificate, TLS certificate verification is skipped, set config.verifySSL = false
+    // 3. If the MinIO server uses a CA-signed certificate, set config.verifySSL = true.
+    config.verifySSL = false;
     if (storage_config.useSSL) {
         config.scheme = Aws::Http::Scheme::HTTPS;
+        if (!storage_config.sslCACert.empty()) {
+            config.verifySSL = true; // uses a CA-signed certificate
+            config.caPath = ConvertToAwsString(storage_config.sslCACert);
+        }
     } else {
-        config.scheme = Aws::Http::Scheme::HTTP;
+        config.scheme = Aws::Http::Scheme::HTTP; // SSL is disabled
     }
 
-    if (!storage_config.sslCACert.empty()) {
-        config.caPath = ConvertToAwsString(storage_config.sslCACert);
-    }
-    config.verifySSL = false;
-
     config.requestTimeoutMs = storage_config.requestTimeoutMs == 0
                                   ? DEFAULT_CHUNK_MANAGER_REQUEST_TIMEOUT_MS
                                   : storage_config.requestTimeoutMs;

diff --git a/internal/proxy/accesslog/minio_handler.go b/internal/proxy/accesslog/minio_handler.go
@@ -108,21 +108,43 @@ func newMinioClient(ctx context.Context, cfg config) (*minio.Client, error) {
 		creds = credentials.NewStaticV4(cfg.accessKeyID, cfg.secretAccessKeyID, "")
 	}
 
+	// We must set the cert path by os environment variable "SSL_CERT_FILE",
+	// because the minio.DefaultTransport() need this path to read the file content,
+	// we shouldn't read this file by ourself.
 	if cfg.useSSL && len(cfg.sslCACert) > 0 {
 		err := os.Setenv("SSL_CERT_FILE", cfg.sslCACert)
 		if err != nil {
 			return nil, err
 		}
 	}
 
+	// The minio.DefaultTransport() creates a default options object for minio.New()
+	// f cfg.useSSL is true, the tr.TLSClientConfig is constructed with os environment variable "SSL_CERT_FILE"
+	tr, err := minio.DefaultTransport(cfg.useSSL)
+	if err != nil {
+		return nil, err
+	}
+
+	// Three cases:
+	// 1. If the MinIO server SSL is disabled, set tr.TLSClientConfig.InsecureSkipVerify = false
+	// 2. If the MinIO server is self-signed certificate, set tr.TLSClientConfig.InsecureSkipVerify = false
+	// 3. If the MinIO server uses a CA-signed certificate, set tr.TLSClientConfig.InsecureSkipVerify = true.
+	tr.TLSClientConfig.InsecureSkipVerify = false
+	if cfg.useSSL && len(cfg.sslCACert) > 0 {
+		tr.TLSClientConfig.InsecureSkipVerify = true
+	}
+
+	// Pass the customized Transport to options
 	minioClient, err := minio.New(cfg.address, &minio.Options{
-		Creds:  creds,
-		Secure: cfg.useSSL,
+		Creds:     creds,
+		Secure:    cfg.useSSL,
+		Transport: tr,
 	})
 	// options nil or invalid formatted endpoint, don't need to retry
 	if err != nil {
 		return nil, err
 	}
+
 	var bucketExists bool
 	// check valid in first query
 	checkBucketFn := func() error {

diff --git a/internal/storage/minio_object_storage.go b/internal/storage/minio_object_storage.go
@@ -107,17 +107,38 @@ func newMinioClient(ctx context.Context, c *config) (*minio.Client, error) {
 		}
 	}
 
+	// We must set the cert path by os environment variable "SSL_CERT_FILE",
+	// because the minio.DefaultTransport() need this path to read the file content,
+	// we shouldn't read this file by ourself.
 	if c.useSSL && len(c.sslCACert) > 0 {
 		err := os.Setenv("SSL_CERT_FILE", c.sslCACert)
 		if err != nil {
 			return nil, err
 		}
 	}
 
+	// The minio.DefaultTransport() creates a default options object for minio.New()
+	// f cfg.useSSL is true, the tr.TLSClientConfig is constructed with os environment variable "SSL_CERT_FILE"
+	tr, err := minio.DefaultTransport(c.useSSL)
+	if err != nil {
+		return nil, err
+	}
+
+	// Three cases:
+	// 1. If the MinIO server SSL is disabled, set tr.TLSClientConfig.InsecureSkipVerify = false
+	// 2. If the MinIO server is self-signed certificate, set tr.TLSClientConfig.InsecureSkipVerify = false
+	// 3. If the MinIO server uses a CA-signed certificate, set tr.TLSClientConfig.InsecureSkipVerify = true.
+	tr.TLSClientConfig.InsecureSkipVerify = false
+	if c.useSSL && len(c.sslCACert) > 0 {
+		tr.TLSClientConfig.InsecureSkipVerify = true
+	}
+
+	// Pass the customized Transport to options
 	minioOpts := &minio.Options{
 		BucketLookup: bucketLookupType,
 		Creds:        creds,
 		Secure:       c.useSSL,
+		Transport:    tr,
 		Region:       c.region,
 	}
 	minIOClient, err := newMinioFn(c.address, minioOpts)

diff --git a/pkg/util/paramtable/service_param.go b/pkg/util/paramtable/service_param.go
@@ -1095,9 +1095,8 @@ func (p *MinioConfig) Init(base *BaseTable) {
 	p.SecretAccessKey.Init(base.mgr)
 
 	p.UseSSL = ParamItem{
-		Key:          "minio.ssl.enabled",
-		FallbackKeys: []string{"minio.useSSL"},
-		Version:      "2.3.12",
+		Key:          "minio.useSSL",
+		Version:      "2.0.0",
 		DefaultValue: "false",
 		PanicIfEmpty: true,
 		Doc:          "Access to MinIO/S3 with SSL",