diff --git a/pkg/operator/ceph/object/bucket/provisioner.go b/pkg/operator/ceph/object/bucket/provisioner.go index 7743f3218bc0..a1d773c9eef3 100644 --- a/pkg/operator/ceph/object/bucket/provisioner.go +++ b/pkg/operator/ceph/object/bucket/provisioner.go @@ -621,7 +621,8 @@ func (p *Provisioner) setAdminOpsAPIClient() error { Timeout: cephObject.HttpTimeOut, } if p.tlsCert != nil { - httpClient.Transport = cephObject.BuildTransportTLS(p.tlsCert) + insecure := false + httpClient.Transport = cephObject.BuildTransportTLS(p.tlsCert, insecure) } // Fetch the ceph object store diff --git a/pkg/operator/ceph/object/health.go b/pkg/operator/ceph/object/health.go index c2963902ae2e..996686d24c85 100644 --- a/pkg/operator/ceph/object/health.go +++ b/pkg/operator/ceph/object/health.go @@ -159,14 +159,13 @@ func (c *bucketChecker) checkObjectStoreHealth() error { } // Set access and secret key - tlsCert := c.objContext.TlsCert s3endpoint := c.objContext.Endpoint s3AccessKey := user.Keys[0].AccessKey s3SecretKey := user.Keys[0].SecretKey // Initiate s3 agent logger.Debugf("initializing s3 connection for object store %q", c.namespacedName.Name) - s3client, err := NewS3Agent(s3AccessKey, s3SecretKey, s3endpoint, "", false, tlsCert) + s3client, err := NewInsecureS3Agent(s3AccessKey, s3SecretKey, s3endpoint, "", false) if err != nil { return errors.Wrap(err, "failed to initialize s3 connection") } diff --git a/pkg/operator/ceph/object/rgw.go b/pkg/operator/ceph/object/rgw.go index 3c9c0248823e..7e89c290f7d9 100644 --- a/pkg/operator/ceph/object/rgw.go +++ b/pkg/operator/ceph/object/rgw.go @@ -361,7 +361,8 @@ func genObjectStoreHTTPClient(objContext *Context, spec *cephv1.ObjectStoreSpec) if err != nil { return nil, tlsCert, errors.Wrapf(err, "failed to fetch CA cert to establish TLS connection with object store %q", nsName) } - c.Transport = BuildTransportTLS(tlsCert) + insecure := false + c.Transport = BuildTransportTLS(tlsCert, insecure) } return c, tlsCert, nil } diff --git a/pkg/operator/ceph/object/s3-handlers.go b/pkg/operator/ceph/object/s3-handlers.go index 74b8b76c1ae9..a76d40879ee7 100644 --- a/pkg/operator/ceph/object/s3-handlers.go +++ b/pkg/operator/ceph/object/s3-handlers.go @@ -1,5 +1,5 @@ /* -Copyright 2018 The Kubernetes Authors. +Copyright 2018 The Rook Authors. All rights reserved. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -40,7 +40,7 @@ func NewS3Agent(accessKey, secretKey, endpoint, region string, debug bool, tlsCe return newS3Agent(accessKey, secretKey, endpoint, region, debug, tlsCert, false) } -func NewTestOnlyS3Agent(accessKey, secretKey, endpoint, region string, debug bool) (*S3Agent, error) { +func NewInsecureS3Agent(accessKey, secretKey, endpoint, region string, debug bool) (*S3Agent, error) { return newS3Agent(accessKey, secretKey, endpoint, region, debug, nil, true) } @@ -60,14 +60,7 @@ func newS3Agent(accessKey, secretKey, endpoint, region string, debug bool, tlsCe tlsEnabled := false if len(tlsCert) > 0 || insecure { tlsEnabled = true - if len(tlsCert) > 0 { - client.Transport = BuildTransportTLS(tlsCert) - } else if insecure { - client.Transport = &http.Transport{ - // #nosec G402 is enabled only for testing - TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, - } - } + client.Transport = BuildTransportTLS(tlsCert, insecure) } sess, err := session.NewSession( aws.NewConfig(). @@ -205,11 +198,16 @@ func (s *S3Agent) DeleteObjectInBucket(bucketname string, key string) (bool, err return true, nil } -func BuildTransportTLS(tlsCert []byte) *http.Transport { - caCertPool := x509.NewCertPool() - caCertPool.AppendCertsFromPEM(tlsCert) +func BuildTransportTLS(tlsCert []byte, insecure bool) *http.Transport { + // #nosec G402 is enabled only for testing + tlsConfig := &tls.Config{MinVersion: tls.VersionTLS12, InsecureSkipVerify: insecure} + if len(tlsCert) > 0 { + caCertPool := x509.NewCertPool() + caCertPool.AppendCertsFromPEM(tlsCert) + tlsConfig.RootCAs = caCertPool + } return &http.Transport{ - TLSClientConfig: &tls.Config{RootCAs: caCertPool, MinVersion: tls.VersionTLS12}, + TLSClientConfig: tlsConfig, } } diff --git a/pkg/operator/ceph/object/s3-handlers_test.go b/pkg/operator/ceph/object/s3-handlers_test.go new file mode 100644 index 000000000000..0417d7eb2549 --- /dev/null +++ b/pkg/operator/ceph/object/s3-handlers_test.go @@ -0,0 +1,80 @@ +/* +Copyright 2021 The Rook Authors. All rights reserved. + +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 object + +import ( + "net/http" + "testing" + + "github.com/aws/aws-sdk-go/aws" + "github.com/stretchr/testify/assert" +) + +func TestNewS3Agent(t *testing.T) { + accessKey := "accessKey" + secretKey := "secretKey" + endpoint := "endpoint" + region := "region" + + t.Run("test without tls/debug", func(t *testing.T) { + debug := false + insecure := false + s3Agent, err := newS3Agent(accessKey, secretKey, endpoint, region, debug, nil, insecure) + assert.NoError(t, err) + assert.NotEqual(t, aws.LogDebug, s3Agent.Client.Config.LogLevel) + assert.Equal(t, nil, s3Agent.Client.Config.HTTPClient.Transport) + assert.True(t, *s3Agent.Client.Config.DisableSSL) + }) + t.Run("test with debug without tls", func(t *testing.T) { + debug := true + logLevel := aws.LogDebug + insecure := false + s3Agent, err := newS3Agent(accessKey, secretKey, endpoint, region, debug, nil, insecure) + assert.NoError(t, err) + assert.Equal(t, &logLevel, s3Agent.Client.Config.LogLevel) + assert.Nil(t, s3Agent.Client.Config.HTTPClient.Transport) + assert.True(t, *s3Agent.Client.Config.DisableSSL) + }) + t.Run("test without tls client cert but insecure tls", func(t *testing.T) { + debug := true + insecure := true + s3Agent, err := newS3Agent(accessKey, secretKey, endpoint, region, debug, nil, insecure) + assert.NoError(t, err) + assert.Nil(t, s3Agent.Client.Config.HTTPClient.Transport.(*http.Transport).TLSClientConfig.RootCAs) + assert.True(t, s3Agent.Client.Config.HTTPClient.Transport.(*http.Transport).TLSClientConfig.InsecureSkipVerify) + assert.False(t, *s3Agent.Client.Config.DisableSSL) + }) + t.Run("test with secure tls client cert", func(t *testing.T) { + debug := true + insecure := false + tlsCert := []byte("tlsCert") + s3Agent, err := newS3Agent(accessKey, secretKey, endpoint, region, debug, tlsCert, insecure) + assert.NoError(t, err) + assert.NotNil(t, s3Agent.Client.Config.HTTPClient.Transport.(*http.Transport).TLSClientConfig.RootCAs) + assert.False(t, *s3Agent.Client.Config.DisableSSL) + }) + t.Run("test with insesure tls client cert", func(t *testing.T) { + debug := true + insecure := true + tlsCert := []byte("tlsCert") + s3Agent, err := newS3Agent(accessKey, secretKey, endpoint, region, debug, tlsCert, insecure) + assert.NoError(t, err) + assert.NotNil(t, s3Agent.Client.Config.HTTPClient.Transport) + assert.True(t, s3Agent.Client.Config.HTTPClient.Transport.(*http.Transport).TLSClientConfig.InsecureSkipVerify) + assert.False(t, *s3Agent.Client.Config.DisableSSL) + }) +} diff --git a/tests/integration/ceph_base_object_test.go b/tests/integration/ceph_base_object_test.go index 61fb518ec152..4ae3a4687014 100644 --- a/tests/integration/ceph_base_object_test.go +++ b/tests/integration/ceph_base_object_test.go @@ -38,6 +38,11 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) +const ( + // #nosec G101 since this is not leaking any hardcoded credentials, it's just the secret name + objectTLSSecretName = "rook-ceph-rgw-tls-test-store-csr" +) + var ( userid = "rook-user" userdisplayname = "A rook RGW user" diff --git a/tests/integration/ceph_object_test.go b/tests/integration/ceph_object_test.go index ef1eff90a950..a342c45e5a1d 100644 --- a/tests/integration/ceph_object_test.go +++ b/tests/integration/ceph_object_test.go @@ -31,9 +31,19 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/suite" v1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) +const ( + objectStoreServicePrefixUniq = "rook-ceph-rgw-" + objectStoreTLSName = "tls-test-store" +) + +var ( + objectStoreServicePrefix = "rook-ceph-rgw-" +) + func TestCephObjectSuite(t *testing.T) { if installer.SkipTestSuite(installer.CephTestSuite) { t.Skip() @@ -92,7 +102,15 @@ func (s *ObjectSuite) TestWithTLS() { } tls := true + objectStoreServicePrefix = objectStoreServicePrefixUniq runObjectE2ETest(s.helper, s.k8sh, s.Suite, s.settings.Namespace, tls) + err := s.k8sh.Clientset.CoreV1().Secrets(s.settings.Namespace).Delete(context.TODO(), objectTLSSecretName, metav1.DeleteOptions{}) + if err != nil { + if !errors.IsNotFound(err) { + logger.Fatal("failed to deleted store TLS secret") + } + } + logger.Info("successfully deleted store TLS secret") } func (s *ObjectSuite) TestWithoutTLS() { @@ -101,6 +119,7 @@ func (s *ObjectSuite) TestWithoutTLS() { } tls := false + objectStoreServicePrefix = objectStoreServicePrefixUniq runObjectE2ETest(s.helper, s.k8sh, s.Suite, s.settings.Namespace, tls) } @@ -111,7 +130,7 @@ func (s *ObjectSuite) TestWithoutTLS() { func runObjectE2ETest(helper *clients.TestClient, k8sh *utils.K8sHelper, s suite.Suite, namespace string, tlsEnable bool) { storeName := "test-store" if tlsEnable { - storeName = "tls-test-store" + storeName = objectStoreTLSName } logger.Infof("Running on Rook Cluster %s", namespace) @@ -125,6 +144,18 @@ func runObjectE2ETest(helper *clients.TestClient, k8sh *utils.K8sHelper, s suite deleteStore := true runObjectE2ETestLite(t, helper, k8sh, namespace, otherStoreName, 1, deleteStore, tlsEnable) }) + if tlsEnable { + // test that a third object store can be created (and deleted) while the first exists + s.T().Run("run a third object store with broken tls", func(t *testing.T) { + otherStoreName := "broken-" + storeName + // The lite e2e test is perfect, as it only creates a cluster, checks that it is healthy, + // and then deletes it. + deleteStore := true + objectStoreServicePrefix = objectStoreServicePrefixUniq + runObjectE2ETestLite(t, helper, k8sh, namespace, otherStoreName, 1, deleteStore, tlsEnable) + objectStoreServicePrefix = objectStoreServicePrefixUniq + }) + } // now test operation of the first object store testObjectStoreOperations(s, helper, k8sh, namespace, storeName) @@ -189,7 +220,7 @@ func testObjectStoreOperations(s suite.Suite, helper *clients.TestClient, k8sh * s3AccessKey, _ := helper.BucketClient.GetAccessKey(obcName) s3SecretKey, _ := helper.BucketClient.GetSecretKey(obcName) if objectStore.Spec.IsTLSEnabled() { - s3client, err = rgw.NewTestOnlyS3Agent(s3AccessKey, s3SecretKey, s3endpoint, region, true) + s3client, err = rgw.NewInsecureS3Agent(s3AccessKey, s3SecretKey, s3endpoint, region, true) } else { s3client, err = rgw.NewS3Agent(s3AccessKey, s3SecretKey, s3endpoint, region, true, nil) }