From 550ab26259735d61cab88dc09071b4c7141361f4 Mon Sep 17 00:00:00 2001 From: yhmo Date: Tue, 26 Mar 2024 10:53:32 +0800 Subject: [PATCH] Fix minio ssl compatible issue Signed-off-by: yhmo --- configs/milvus.yaml | 2 +- internal/core/src/storage/ChunkManager.cpp | 16 +++++++----- .../core/src/storage/MinioChunkManager.cpp | 16 +++++++----- internal/proxy/accesslog/minio_handler.go | 26 +++++++++++++++++-- internal/storage/minio_object_storage.go | 21 +++++++++++++++ pkg/util/paramtable/service_param.go | 5 ++-- 6 files changed, 68 insertions(+), 18 deletions(-) diff --git a/configs/milvus.yaml b/configs/milvus.yaml index f926e564501aa..4cb07ef293baf 100644 --- 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 index 18c3df37ba765..57b7447bedfa5 100644 --- 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 index ca9f6e7cde6a2..565b97ab145d4 100644 --- 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 index dcac92b43a4ea..e6cb53963d388 100644 --- a/internal/proxy/accesslog/minio_handler.go +++ b/internal/proxy/accesslog/minio_handler.go @@ -108,6 +108,9 @@ 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 { @@ -115,14 +118,33 @@ func newMinioClient(ctx context.Context, cfg config) (*minio.Client, error) { } } + // 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 index 11d7389d62ad7..e0cab4848576e 100644 --- a/internal/storage/minio_object_storage.go +++ b/internal/storage/minio_object_storage.go @@ -107,6 +107,9 @@ 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 { @@ -114,10 +117,28 @@ func newMinioClient(ctx context.Context, c *config) (*minio.Client, error) { } } + // 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 index b1516c683d8ed..01edd511cebf3 100644 --- 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",