From 5fb914ac3f8816ea0a828e5037b8e408e492e225 Mon Sep 17 00:00:00 2001 From: Matheus Pimenta Date: Tue, 6 Aug 2024 14:13:01 +0100 Subject: [PATCH] Add LDAP provider for Bucket STS API Signed-off-by: Matheus Pimenta --- api/v1beta2/bucket_types.go | 16 +- api/v1beta2/sts_types.go | 3 + api/v1beta2/zz_generated.deepcopy.go | 7 +- .../source.toolkit.fluxcd.io_buckets.yaml | 29 +- docs/api/v1beta2/source.md | 21 +- docs/spec/v1beta2/buckets.md | 17 +- internal/controller/bucket_controller.go | 40 ++- internal/controller/bucket_controller_test.go | 118 ++++++-- pkg/minio/minio.go | 83 ++++++ pkg/minio/minio_test.go | 258 ++++++++++++++++-- 10 files changed, 519 insertions(+), 73 deletions(-) diff --git a/api/v1beta2/bucket_types.go b/api/v1beta2/bucket_types.go index 3a9efa22d..d2e5a6e6e 100644 --- a/api/v1beta2/bucket_types.go +++ b/api/v1beta2/bucket_types.go @@ -49,8 +49,10 @@ const ( // BucketSpec specifies the required configuration to produce an Artifact for // an object storage bucket. -// +kubebuilder:validation:XValidation:rule="self.provider == 'aws' || !has(self.sts)", message="STS configuration is only supported for the 'aws' Bucket provider" +// +kubebuilder:validation:XValidation:rule="self.provider == 'aws' || self.provider == 'generic' || !has(self.sts)", message="STS configuration is only supported for the 'aws' and 'generic' Bucket providers" // +kubebuilder:validation:XValidation:rule="self.provider != 'aws' || !has(self.sts) || self.sts.provider == 'aws'", message="'aws' is the only supported STS provider for the 'aws' Bucket provider" +// +kubebuilder:validation:XValidation:rule="self.provider != 'generic' || !has(self.sts) || self.sts.provider == 'ldap'", message="'ldap' is the only supported STS provider for the 'generic' Bucket provider" +// +kubebuilder:validation:XValidation:rule="!has(self.sts) || self.sts.provider != 'aws' || !has(self.sts.secretRef)", message="spec.sts.secretRef is not required for the 'aws' STS provider" type BucketSpec struct { // Provider of the object storage bucket. // Defaults to 'generic', which expects an S3 (API) compatible object @@ -72,7 +74,7 @@ type BucketSpec struct { // Service for fetching temporary credentials to authenticate in a // Bucket provider. // - // This field is only supported for the `aws` provider. + // This field is only supported for the `aws` and `generic` providers. // +optional STS *BucketSTSSpec `json:"sts,omitempty"` @@ -153,7 +155,7 @@ type BucketSpec struct { // provider. type BucketSTSSpec struct { // Provider of the Security Token Service. - // +kubebuilder:validation:Enum=aws + // +kubebuilder:validation:Enum=aws;ldap // +required Provider string `json:"provider"` @@ -162,6 +164,14 @@ type BucketSTSSpec struct { // +required // +kubebuilder:validation:Pattern="^(http|https)://.*$" Endpoint string `json:"endpoint"` + + // SecretRef specifies the Secret containing authentication credentials + // for the STS endpoint. + // + // Required for the `ldap` provider, in which case it should contain + // the fields `username` and `password`. + // +optional + SecretRef *meta.LocalObjectReference `json:"secretRef,omitempty"` } // BucketStatus records the observed state of a Bucket. diff --git a/api/v1beta2/sts_types.go b/api/v1beta2/sts_types.go index d9e0b97ef..c07c05123 100644 --- a/api/v1beta2/sts_types.go +++ b/api/v1beta2/sts_types.go @@ -20,4 +20,7 @@ const ( // STSProviderAmazon represents the AWS provider for Security Token Service. // Provides support for fetching temporary credentials from an AWS STS endpoint. STSProviderAmazon string = "aws" + // STSProviderLDAP represents the LDAP provider for Security Token Service. + // Provides support for fetching temporary credentials from an LDAP endpoint. + STSProviderLDAP string = "ldap" ) diff --git a/api/v1beta2/zz_generated.deepcopy.go b/api/v1beta2/zz_generated.deepcopy.go index 1a7c8fc79..5668e952d 100644 --- a/api/v1beta2/zz_generated.deepcopy.go +++ b/api/v1beta2/zz_generated.deepcopy.go @@ -118,6 +118,11 @@ func (in *BucketList) DeepCopyObject() runtime.Object { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *BucketSTSSpec) DeepCopyInto(out *BucketSTSSpec) { *out = *in + if in.SecretRef != nil { + in, out := &in.SecretRef, &out.SecretRef + *out = new(meta.LocalObjectReference) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new BucketSTSSpec. @@ -136,7 +141,7 @@ func (in *BucketSpec) DeepCopyInto(out *BucketSpec) { if in.STS != nil { in, out := &in.STS, &out.STS *out = new(BucketSTSSpec) - **out = **in + (*in).DeepCopyInto(*out) } if in.SecretRef != nil { in, out := &in.SecretRef, &out.SecretRef diff --git a/config/crd/bases/source.toolkit.fluxcd.io_buckets.yaml b/config/crd/bases/source.toolkit.fluxcd.io_buckets.yaml index 7c79930e9..4f98b7023 100644 --- a/config/crd/bases/source.toolkit.fluxcd.io_buckets.yaml +++ b/config/crd/bases/source.toolkit.fluxcd.io_buckets.yaml @@ -424,7 +424,7 @@ spec: Bucket provider. - This field is only supported for the `aws` provider. + This field is only supported for the `aws` and `generic` providers. properties: endpoint: description: |- @@ -436,7 +436,23 @@ spec: description: Provider of the Security Token Service. enum: - aws + - ldap type: string + secretRef: + description: |- + SecretRef specifies the Secret containing authentication credentials + for the STS endpoint. + + + Required for the `ldap` provider, in which case it should contain + the fields `username` and `password`. + properties: + name: + description: Name of the referent. + type: string + required: + - name + type: object required: - endpoint - provider @@ -457,12 +473,19 @@ spec: - interval type: object x-kubernetes-validations: - - message: STS configuration is only supported for the 'aws' Bucket provider - rule: self.provider == 'aws' || !has(self.sts) + - message: STS configuration is only supported for the 'aws' and 'generic' + Bucket providers + rule: self.provider == 'aws' || self.provider == 'generic' || !has(self.sts) - message: '''aws'' is the only supported STS provider for the ''aws'' Bucket provider' rule: self.provider != 'aws' || !has(self.sts) || self.sts.provider == 'aws' + - message: '''ldap'' is the only supported STS provider for the ''generic'' + Bucket provider' + rule: self.provider != 'generic' || !has(self.sts) || self.sts.provider + == 'ldap' + - message: spec.sts.secretRef is not required for the 'aws' STS provider + rule: '!has(self.sts) || self.sts.provider != ''aws'' || !has(self.sts.secretRef)' status: default: observedGeneration: -1 diff --git a/docs/api/v1beta2/source.md b/docs/api/v1beta2/source.md index 10ef720f5..70eab8b51 100644 --- a/docs/api/v1beta2/source.md +++ b/docs/api/v1beta2/source.md @@ -126,7 +126,7 @@ BucketSTSSpec

STS specifies the required configuration to use a Security Token Service for fetching temporary credentials to authenticate in a Bucket provider.

-

This field is only supported for the aws provider.

+

This field is only supported for the aws and generic providers.

@@ -1497,6 +1497,23 @@ string where temporary credentials will be fetched.

+ + +secretRef
+ + +github.com/fluxcd/pkg/apis/meta.LocalObjectReference + + + + +(Optional) +

SecretRef specifies the Secret containing authentication credentials +for the STS endpoint.

+

Required for the ldap provider, in which case it should contain +the fields username and password.

+ + @@ -1569,7 +1586,7 @@ BucketSTSSpec

STS specifies the required configuration to use a Security Token Service for fetching temporary credentials to authenticate in a Bucket provider.

-

This field is only supported for the aws provider.

+

This field is only supported for the aws and generic providers.

diff --git a/docs/spec/v1beta2/buckets.md b/docs/spec/v1beta2/buckets.md index b84623468..eb61ed742 100644 --- a/docs/spec/v1beta2/buckets.md +++ b/docs/spec/v1beta2/buckets.md @@ -756,15 +756,24 @@ configuration. A Security Token Service (STS) is a web service that issues temporary security credentials. By adding this field, one may specify the STS endpoint from where temporary credentials will be fetched. +This field is only supported for the `aws` and `generic` bucket [providers](#provider). + If using `.spec.sts`, the following fields are required: - `.spec.sts.provider`, the Security Token Service provider. The only supported - option is `aws`. + option for the `aws` bucket provider is `aws`. The only supported option for + the `generic` bucket provider is `ldap`. - `.spec.sts.endpoint`, the HTTP/S endpoint of the Security Token Service. In - the case of AWS, this can be `https://sts.amazonaws.com`, or a Regional STS - Endpoint, or an Interface Endpoint created inside a VPC. + the case of `aws` this can be `https://sts.amazonaws.com`, or a Regional STS + Endpoint, or an Interface Endpoint created inside a VPC. In the case of + `ldap` this must be the LDAP server endpoint. + +When using the `ldap` provider, the following fields are also required: -This field is only supported for the `aws` bucket provider. +- `.spec.sts.secretRef.name`, the name of the Secret containing the LDAP + credentials. The Secret must contain the following keys: + - `username`, the username to authenticate with. + - `password`, the password to authenticate with. ### Bucket name diff --git a/internal/controller/bucket_controller.go b/internal/controller/bucket_controller.go index 6fbaf0129..b434f3c2a 100644 --- a/internal/controller/bucket_controller.go +++ b/internal/controller/bucket_controller.go @@ -428,7 +428,6 @@ func (r *BucketReconciler) reconcileSource(ctx context.Context, sp *patch.Serial if err != nil { e := serror.NewGeneric(err, sourcev1.AuthenticationFailedReason) conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, "%s", e) - // Return error as the world as observed may change return sreconcile.ResultEmpty, e } proxyURL, err := r.getProxyURL(ctx, obj) @@ -483,6 +482,18 @@ func (r *BucketReconciler) reconcileSource(ctx context.Context, sp *patch.Serial conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, "%s", e) return sreconcile.ResultEmpty, e } + tlsConfig, err := r.getTLSConfig(ctx, obj) + if err != nil { + e := serror.NewGeneric(err, sourcev1.AuthenticationFailedReason) + conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, "%s", e) + return sreconcile.ResultEmpty, e + } + stsSecret, err := r.getSTSSecret(ctx, obj) + if err != nil { + e := serror.NewGeneric(err, sourcev1.AuthenticationFailedReason) + conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, "%s", e) + return sreconcile.ResultEmpty, e + } if sts := obj.Spec.STS; sts != nil { if err := minio.ValidateSTSProvider(obj.Spec.Provider, sts.Provider); err != nil { e := serror.NewStalling(err, sourcev1.InvalidSTSConfigurationReason) @@ -495,12 +506,11 @@ func (r *BucketReconciler) reconcileSource(ctx context.Context, sp *patch.Serial conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, "%s", e) return sreconcile.ResultEmpty, e } - } - tlsConfig, err := r.getTLSConfig(ctx, obj) - if err != nil { - e := serror.NewGeneric(err, sourcev1.AuthenticationFailedReason) - conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, "%s", e) - return sreconcile.ResultEmpty, e + if err := minio.ValidateSTSSecret(sts.Provider, stsSecret); err != nil { + e := serror.NewGeneric(err, sourcev1.AuthenticationFailedReason) + conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, "%s", e) + return sreconcile.ResultEmpty, e + } } var opts []minio.Option if secret != nil { @@ -512,6 +522,9 @@ func (r *BucketReconciler) reconcileSource(ctx context.Context, sp *patch.Serial if proxyURL != nil { opts = append(opts, minio.WithProxyURL(proxyURL)) } + if stsSecret != nil { + opts = append(opts, minio.WithSTSSecret(stsSecret)) + } if provider, err = minio.NewClient(obj, opts...); err != nil { e := serror.NewGeneric(err, "ClientError") conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, "%s", e) @@ -732,6 +745,8 @@ func (r *BucketReconciler) getSecret(ctx context.Context, secretRef *meta.LocalO return secret, nil } +// getTLSConfig attempts to fetch a TLS configuration from the object's +// certificate secret reference. func (r *BucketReconciler) getTLSConfig(ctx context.Context, obj *bucketv1.Bucket) (*stdtls.Config, error) { certSecret, err := r.getSecret(ctx, obj.Spec.CertSecretRef, obj.GetNamespace()) if err != nil || certSecret == nil { @@ -747,6 +762,8 @@ func (r *BucketReconciler) getTLSConfig(ctx context.Context, obj *bucketv1.Bucke return tlsConfig, nil } +// getProxyURL attempts to fetch a proxy URL from the object's proxy secret +// reference. func (r *BucketReconciler) getProxyURL(ctx context.Context, obj *bucketv1.Bucket) (*url.URL, error) { namespace := obj.GetNamespace() proxySecret, err := r.getSecret(ctx, obj.Spec.ProxySecretRef, namespace) @@ -771,6 +788,15 @@ func (r *BucketReconciler) getProxyURL(ctx context.Context, obj *bucketv1.Bucket return proxyURL, nil } +// getSTSSecret attempts to fetch the secret from the object's STS secret +// reference. +func (r *BucketReconciler) getSTSSecret(ctx context.Context, obj *bucketv1.Bucket) (*corev1.Secret, error) { + if obj.Spec.STS == nil { + return nil, nil + } + return r.getSecret(ctx, obj.Spec.STS.SecretRef, obj.GetNamespace()) +} + // eventLogf records events, and logs at the same time. // // This log is different from the debug log in the EventRecorder, in the sense diff --git a/internal/controller/bucket_controller_test.go b/internal/controller/bucket_controller_test.go index aa710edbc..23ec2e9a9 100644 --- a/internal/controller/bucket_controller_test.go +++ b/internal/controller/bucket_controller_test.go @@ -592,6 +592,50 @@ func TestBucketReconciler_reconcileSource_generic(t *testing.T) { *conditions.TrueCondition(sourcev1.FetchFailedCondition, sourcev1.AuthenticationFailedReason, "invalid proxy secret '/dummy': key 'address' is missing"), }, }, + { + name: "Observes non-existing sts.secretRef", + bucketName: "dummy", + beforeFunc: func(obj *bucketv1.Bucket) { + obj.Spec.STS = &bucketv1.BucketSTSSpec{ + SecretRef: &meta.LocalObjectReference{Name: "dummy"}, + } + conditions.MarkReconciling(obj, meta.ProgressingReason, "foo") + conditions.MarkUnknown(obj, meta.ReadyCondition, "foo", "bar") + }, + wantErr: true, + assertIndex: index.NewDigester(), + assertConditions: []metav1.Condition{ + *conditions.TrueCondition(sourcev1.FetchFailedCondition, sourcev1.AuthenticationFailedReason, "failed to get secret '/dummy': secrets \"dummy\" not found"), + *conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "foo"), + *conditions.UnknownCondition(meta.ReadyCondition, "foo", "bar"), + }, + }, + { + name: "Observes invalid STS secret data", + bucketName: "dummy", + secret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "dummy", + }, + }, + beforeFunc: func(obj *bucketv1.Bucket) { + obj.Spec.Provider = "generic" + obj.Spec.STS = &bucketv1.BucketSTSSpec{ + Provider: "ldap", + Endpoint: "https://something", + SecretRef: &meta.LocalObjectReference{Name: "dummy"}, + } + conditions.MarkReconciling(obj, meta.ProgressingReason, "foo") + conditions.MarkUnknown(obj, meta.ReadyCondition, "foo", "bar") + }, + wantErr: true, + assertIndex: index.NewDigester(), + assertConditions: []metav1.Condition{ + *conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "foo"), + *conditions.UnknownCondition(meta.ReadyCondition, "foo", "bar"), + *conditions.TrueCondition(sourcev1.FetchFailedCondition, sourcev1.AuthenticationFailedReason, "invalid 'dummy' secret data for 'ldap' STS provider: required fields username, password"), + }, + }, { name: "Observes non-existing bucket name", bucketName: "dummy", @@ -622,7 +666,7 @@ func TestBucketReconciler_reconcileSource_generic(t *testing.T) { wantErr: true, assertIndex: index.NewDigester(), assertConditions: []metav1.Condition{ - *conditions.TrueCondition(sourcev1.FetchFailedCondition, sourcev1.InvalidSTSConfigurationReason, "STS configuration is not supported for 'generic' bucket provider"), + *conditions.TrueCondition(sourcev1.FetchFailedCondition, sourcev1.InvalidSTSConfigurationReason, "STS provider 'aws' is not supported for 'generic' bucket provider"), *conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "foo"), *conditions.UnknownCondition(meta.ReadyCondition, "foo", "bar"), }, @@ -631,9 +675,9 @@ func TestBucketReconciler_reconcileSource_generic(t *testing.T) { name: "Observes invalid STS endpoint", bucketName: "dummy", beforeFunc: func(obj *bucketv1.Bucket) { - obj.Spec.Provider = "aws" // TODO: change to generic when ldap STS provider is implemented + obj.Spec.Provider = "generic" obj.Spec.STS = &bucketv1.BucketSTSSpec{ - Provider: "aws", // TODO: change to ldap when ldap STS provider is implemented + Provider: "ldap", Endpoint: "something\t", } conditions.MarkReconciling(obj, meta.ProgressingReason, "foo") @@ -1863,7 +1907,7 @@ func TestBucketReconciler_APIServerValidation_STS(t *testing.T) { Provider: "aws", Endpoint: "http://test", }, - err: "STS configuration is only supported for the 'aws' Bucket provider", + err: "STS configuration is only supported for the 'aws' and 'generic' Bucket providers", }, { name: "azure unsupported", @@ -1872,16 +1916,7 @@ func TestBucketReconciler_APIServerValidation_STS(t *testing.T) { Provider: "aws", Endpoint: "http://test", }, - err: "STS configuration is only supported for the 'aws' Bucket provider", - }, - { - name: "generic unsupported", - bucketProvider: "generic", - stsConfig: &bucketv1.BucketSTSSpec{ - Provider: "aws", - Endpoint: "http://test", - }, - err: "STS configuration is only supported for the 'aws' Bucket provider", + err: "STS configuration is only supported for the 'aws' and 'generic' Bucket providers", }, { name: "aws supported", @@ -1916,16 +1951,51 @@ func TestBucketReconciler_APIServerValidation_STS(t *testing.T) { name: "aws can be created without STS config", bucketProvider: "aws", }, - // Can't be tested at present with only one allowed sts provider. - // { - // name: "ldap unsupported for aws", - // bucketProvider: "aws", - // stsConfig: &bucketv1.BucketSTSSpec{ - // Provider: "ldap", - // Endpoint: "http://test", - // }, - // err: "'aws' is the only supported STS provider for the 'aws' Bucket provider", - // }, + { + name: "ldap unsupported for aws", + bucketProvider: "aws", + stsConfig: &bucketv1.BucketSTSSpec{ + Provider: "ldap", + Endpoint: "http://test", + }, + err: "'aws' is the only supported STS provider for the 'aws' Bucket provider", + }, + { + name: "aws unsupported for generic", + bucketProvider: "generic", + stsConfig: &bucketv1.BucketSTSSpec{ + Provider: "aws", + Endpoint: "http://test", + }, + err: "'ldap' is the only supported STS provider for the 'generic' Bucket provider", + }, + { + name: "aws does not require a secret", + bucketProvider: "aws", + stsConfig: &bucketv1.BucketSTSSpec{ + Provider: "aws", + Endpoint: "http://test", + SecretRef: &meta.LocalObjectReference{}, + }, + err: "spec.sts.secretRef is not required for the 'aws' STS provider", + }, + { + name: "ldap may use a secret", + bucketProvider: "generic", + stsConfig: &bucketv1.BucketSTSSpec{ + Provider: "ldap", + Endpoint: "http://test", + SecretRef: &meta.LocalObjectReference{}, + }, + }, + { + name: "ldap may not use a secret", + bucketProvider: "generic", + stsConfig: &bucketv1.BucketSTSSpec{ + Provider: "ldap", + Endpoint: "http://test", + }, + }, } for _, tt := range tests { diff --git a/pkg/minio/minio.go b/pkg/minio/minio.go index 604ef1de6..961ae162c 100644 --- a/pkg/minio/minio.go +++ b/pkg/minio/minio.go @@ -23,6 +23,7 @@ import ( "fmt" "net/http" "net/url" + "strings" "github.com/minio/minio-go/v7" "github.com/minio/minio-go/v7/pkg/credentials" @@ -41,6 +42,7 @@ type MinioClient struct { // options holds the configuration for the Minio client. type options struct { secret *corev1.Secret + stsSecret *corev1.Secret tlsConfig *tls.Config proxyURL *url.URL } @@ -69,6 +71,13 @@ func WithProxyURL(proxyURL *url.URL) Option { } } +// WithSTSSecret sets the STS secret for the Minio client. +func WithSTSSecret(secret *corev1.Secret) Option { + return func(o *options) { + o.stsSecret = secret + } +} + // NewClient creates a new Minio storage client. func NewClient(bucket *sourcev1.Bucket, opts ...Option) (*MinioClient, error) { var o options @@ -89,6 +98,8 @@ func NewClient(bucket *sourcev1.Bucket, opts ...Option) (*MinioClient, error) { minioOpts.Creds = newCredsFromSecret(o.secret) case bucketProvider == sourcev1.AmazonBucketProvider: minioOpts.Creds = newAWSCreds(bucket, o.proxyURL) + case bucketProvider == sourcev1.GenericBucketProvider: + minioOpts.Creds = newGenericCreds(bucket, &o) } var transportOpts []func(*http.Transport) @@ -159,6 +170,38 @@ func newAWSCreds(bucket *sourcev1.Bucket, proxyURL *url.URL) *credentials.Creden return creds } +// newGenericCreds creates a new Minio credentials object for the `generic` bucket provider. +func newGenericCreds(bucket *sourcev1.Bucket, o *options) *credentials.Credentials { + + sts := bucket.Spec.STS + if sts == nil { + return nil + } + + switch sts.Provider { + case sourcev1.STSProviderLDAP: + client := &http.Client{Transport: http.DefaultTransport} + if o.proxyURL != nil { + transport := http.DefaultTransport.(*http.Transport).Clone() + transport.Proxy = http.ProxyURL(o.proxyURL) + client = &http.Client{Transport: transport} + } + var username, password string + if o.stsSecret != nil { + username = string(o.stsSecret.Data["username"]) + password = string(o.stsSecret.Data["password"]) + } + return credentials.New(&credentials.LDAPIdentity{ + Client: client, + STSEndpoint: sts.Endpoint, + LDAPUsername: username, + LDAPPassword: password, + }) + } + + return nil +} + // ValidateSecret validates the credential secret. The provided Secret may // be nil. func ValidateSecret(secret *corev1.Secret) error { @@ -188,11 +231,51 @@ func ValidateSTSProvider(bucketProvider, stsProvider string) error { default: return errProviderIncompatbility } + case sourcev1.GenericBucketProvider: + switch stsProvider { + case sourcev1.STSProviderLDAP: + return nil + default: + return errProviderIncompatbility + } } return fmt.Errorf("STS configuration is not supported for '%s' bucket provider", bucketProvider) } +// ValidateSTSSecret validates the STS secret. The provided Secret may be nil. +func ValidateSTSSecret(stsProvider string, secret *corev1.Secret) error { + switch stsProvider { + case sourcev1.STSProviderLDAP: + return validateSTSSecretForProvider(stsProvider, secret, "username", "password") + } + + if secret != nil { + return fmt.Errorf("STS provider '%s' does not require a secret", stsProvider) + } + return nil +} + +// validateSTSSecretForProvider validates the STS secret for each provider. +// The provided Secret may be nil. +func validateSTSSecretForProvider(stsProvider string, secret *corev1.Secret, keys ...string) error { + if secret == nil { + return nil + } + err := fmt.Errorf("invalid '%s' secret data for '%s' STS provider: required fields %s", + secret.Name, stsProvider, strings.Join(keys, ", ")) + if len(secret.Data) == 0 { + return err + } + for _, key := range keys { + value, ok := secret.Data[key] + if !ok || len(value) == 0 { + return err + } + } + return nil +} + // FGetObject gets the object from the provided object storage bucket, and // writes it to targetPath. // It returns the etag of the successfully fetched file, or any error. diff --git a/pkg/minio/minio_test.go b/pkg/minio/minio_test.go index c48f09b5f..70b734bac 100644 --- a/pkg/minio/minio_test.go +++ b/pkg/minio/minio_test.go @@ -21,6 +21,7 @@ import ( "crypto/tls" "crypto/x509" "encoding/json" + "encoding/xml" "errors" "fmt" "log" @@ -247,24 +248,24 @@ func TestFGetObject(t *testing.T) { } func TestNewClientAndFGetObjectWithSTSEndpoint(t *testing.T) { - // start a mock STS server - stsListener, stsAddr, stsPort := testlistener.New(t) - stsEndpoint := fmt.Sprintf("http://%s", stsAddr) - stsHandler := http.NewServeMux() - stsHandler.HandleFunc("PUT "+credentials.TokenPath, + // start a mock AWS STS server + awsSTSListener, awsSTSAddr, awsSTSPort := testlistener.New(t) + awsSTSEndpoint := fmt.Sprintf("http://%s", awsSTSAddr) + awsSTSHandler := http.NewServeMux() + awsSTSHandler.HandleFunc("PUT "+credentials.TokenPath, func(w http.ResponseWriter, r *http.Request) { _, err := w.Write([]byte("mock-token")) assert.NilError(t, err) }) - stsHandler.HandleFunc("GET "+credentials.DefaultIAMSecurityCredsPath, + awsSTSHandler.HandleFunc("GET "+credentials.DefaultIAMSecurityCredsPath, func(w http.ResponseWriter, r *http.Request) { token := r.Header.Get(credentials.TokenRequestHeader) assert.Equal(t, token, "mock-token") _, err := w.Write([]byte("mock-role")) assert.NilError(t, err) }) - var roleCredsRetrieved bool - stsHandler.HandleFunc("GET "+credentials.DefaultIAMSecurityCredsPath+"mock-role", + var credsRetrieved bool + awsSTSHandler.HandleFunc("GET "+credentials.DefaultIAMSecurityCredsPath+"mock-role", func(w http.ResponseWriter, r *http.Request) { token := r.Header.Get(credentials.TokenRequestHeader) assert.Equal(t, token, "mock-token") @@ -274,66 +275,162 @@ func TestNewClientAndFGetObjectWithSTSEndpoint(t *testing.T) { "SecretAccessKey": testMinioRootPassword, }) assert.NilError(t, err) - roleCredsRetrieved = true + credsRetrieved = true }) - stsServer := &http.Server{ - Addr: stsAddr, - Handler: stsHandler, + awsSTSServer := &http.Server{ + Addr: awsSTSAddr, + Handler: awsSTSHandler, } - go stsServer.Serve(stsListener) - defer stsServer.Shutdown(context.Background()) + go awsSTSServer.Serve(awsSTSListener) + defer awsSTSServer.Shutdown(context.Background()) + + // start a mock LDAP STS server + ldapSTSListener, ldapSTSAddr, ldapSTSPort := testlistener.New(t) + ldapSTSEndpoint := fmt.Sprintf("http://%s", ldapSTSAddr) + ldapSTSHandler := http.NewServeMux() + var ldapUsername, ldapPassword string + ldapSTSHandler.HandleFunc("POST /", + func(w http.ResponseWriter, r *http.Request) { + err := r.ParseForm() + assert.NilError(t, err) + username := r.Form.Get("LDAPUsername") + password := r.Form.Get("LDAPPassword") + fmt.Println(username, password) + assert.Equal(t, username, ldapUsername) + assert.Equal(t, password, ldapPassword) + var result credentials.LDAPIdentityResult + result.Credentials.AccessKey = testMinioRootUser + result.Credentials.SecretKey = testMinioRootPassword + err = xml.NewEncoder(w).Encode(credentials.AssumeRoleWithLDAPResponse{Result: result}) + assert.NilError(t, err) + credsRetrieved = true + }) + ldapSTSServer := &http.Server{ + Addr: ldapSTSAddr, + Handler: ldapSTSHandler, + } + go ldapSTSServer.Serve(ldapSTSListener) + defer ldapSTSServer.Shutdown(context.Background()) // start proxy proxyAddr, proxyPort := testproxy.New(t) tests := []struct { - name string - provider string - stsSpec *sourcev1.BucketSTSSpec - opts []Option - err string + name string + provider string + stsSpec *sourcev1.BucketSTSSpec + opts []Option + ldapUsername string + ldapPassword string + err string }{ { - name: "with correct endpoint", + name: "with correct aws endpoint", provider: "aws", stsSpec: &sourcev1.BucketSTSSpec{ Provider: "aws", - Endpoint: stsEndpoint, + Endpoint: awsSTSEndpoint, }, }, { - name: "with incorrect endpoint", + name: "with incorrect aws endpoint", provider: "aws", stsSpec: &sourcev1.BucketSTSSpec{ Provider: "aws", - Endpoint: fmt.Sprintf("http://localhost:%d", stsPort+1), + Endpoint: fmt.Sprintf("http://localhost:%d", awsSTSPort+1), }, err: "connection refused", }, { - name: "with correct endpoint and proxy", + name: "with correct aws endpoint and proxy", provider: "aws", stsSpec: &sourcev1.BucketSTSSpec{ Provider: "aws", - Endpoint: stsEndpoint, + Endpoint: awsSTSEndpoint, }, opts: []Option{WithProxyURL(&url.URL{Scheme: "http", Host: proxyAddr})}, }, { - name: "with correct endpoint and incorrect proxy", + name: "with correct aws endpoint and incorrect proxy", provider: "aws", stsSpec: &sourcev1.BucketSTSSpec{ Provider: "aws", - Endpoint: stsEndpoint, + Endpoint: awsSTSEndpoint, }, opts: []Option{WithProxyURL(&url.URL{Scheme: "http", Host: fmt.Sprintf("localhost:%d", proxyPort+1)})}, err: "connection refused", }, + { + name: "with correct ldap endpoint", + provider: "generic", + stsSpec: &sourcev1.BucketSTSSpec{ + Provider: "ldap", + Endpoint: ldapSTSEndpoint, + }, + opts: []Option{WithSTSSecret(&corev1.Secret{ + Data: map[string][]byte{ + "username": []byte("user"), + "password": []byte("password"), + }, + })}, + ldapUsername: "user", + ldapPassword: "password", + }, + { + name: "with correct ldap endpoint without secret", + provider: "generic", + stsSpec: &sourcev1.BucketSTSSpec{ + Provider: "ldap", + Endpoint: ldapSTSEndpoint, + }, + }, + { + name: "with incorrect ldap endpoint", + provider: "generic", + stsSpec: &sourcev1.BucketSTSSpec{ + Provider: "ldap", + Endpoint: fmt.Sprintf("http://localhost:%d", ldapSTSPort+1), + }, + err: "connection refused", + }, + { + name: "with correct ldap endpoint and proxy", + provider: "generic", + stsSpec: &sourcev1.BucketSTSSpec{ + Provider: "ldap", + Endpoint: ldapSTSEndpoint, + }, + opts: []Option{ + WithProxyURL(&url.URL{Scheme: "http", Host: proxyAddr}), + WithSTSSecret(&corev1.Secret{ + Data: map[string][]byte{ + "username": []byte("user"), + "password": []byte("password"), + }, + }), + }, + ldapUsername: "user", + ldapPassword: "password", + }, + { + name: "with correct ldap endpoint and incorrect proxy", + provider: "generic", + stsSpec: &sourcev1.BucketSTSSpec{ + Provider: "ldap", + Endpoint: ldapSTSEndpoint, + }, + opts: []Option{ + WithProxyURL(&url.URL{Scheme: "http", Host: fmt.Sprintf("localhost:%d", proxyPort+1)}), + }, + err: "connection refused", + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - roleCredsRetrieved = false + credsRetrieved = false + ldapUsername = tt.ldapUsername + ldapPassword = tt.ldapPassword bucket := bucketStub(bucket, testMinioAddress) bucket.Spec.Provider = tt.provider bucket.Spec.STS = tt.stsSpec @@ -348,7 +445,7 @@ func TestNewClientAndFGetObjectWithSTSEndpoint(t *testing.T) { assert.ErrorContains(t, err, tt.err) } else { assert.NilError(t, err) - assert.Assert(t, roleCredsRetrieved) + assert.Assert(t, credsRetrieved) } }) } @@ -484,12 +581,23 @@ func TestValidateSTSProvider(t *testing.T) { bucketProvider: "aws", stsProvider: "aws", }, + { + name: "ldap", + bucketProvider: "generic", + stsProvider: "ldap", + }, { name: "unsupported for aws", bucketProvider: "aws", stsProvider: "ldap", err: "STS provider 'ldap' is not supported for 'aws' bucket provider", }, + { + name: "unsupported for generic", + bucketProvider: "generic", + stsProvider: "aws", + err: "STS provider 'aws' is not supported for 'generic' bucket provider", + }, { name: "unsupported bucket provider", bucketProvider: "gcp", @@ -511,6 +619,98 @@ func TestValidateSTSProvider(t *testing.T) { } } +func TestValidateSTSSecret(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + provider string + secret *corev1.Secret + err string + }{ + { + name: "aws provider returns error for non-nil secret", + provider: "aws", + secret: &corev1.Secret{}, + err: "STS provider 'aws' does not require a secret", + }, + { + name: "ldap provider does not require a secret", + provider: "ldap", + }, + { + name: "valid ldap secret", + provider: "ldap", + secret: &corev1.Secret{ + Data: map[string][]byte{ + "username": []byte("user"), + "password": []byte("pass"), + }, + }, + }, + { + name: "empty ldap secret", + provider: "ldap", + secret: &corev1.Secret{ObjectMeta: v1.ObjectMeta{Name: "ldap-secret"}}, + err: "invalid 'ldap-secret' secret data for 'ldap' STS provider: required fields username, password", + }, + { + name: "ldap secret missing password", + provider: "ldap", + secret: &corev1.Secret{ + Data: map[string][]byte{ + "username": []byte("user"), + }, + }, + err: "invalid '' secret data for 'ldap' STS provider: required fields username, password", + }, + { + name: "ldap secret missing username", + provider: "ldap", + secret: &corev1.Secret{ + Data: map[string][]byte{ + "password": []byte("pass"), + }, + }, + err: "invalid '' secret data for 'ldap' STS provider: required fields username, password", + }, + { + name: "ldap secret with empty username", + provider: "ldap", + secret: &corev1.Secret{ + Data: map[string][]byte{ + "username": []byte(""), + "password": []byte("pass"), + }, + }, + err: "invalid '' secret data for 'ldap' STS provider: required fields username, password", + }, + { + name: "ldap secret with empty password", + provider: "ldap", + secret: &corev1.Secret{ + Data: map[string][]byte{ + "username": []byte("user"), + "password": []byte(""), + }, + }, + err: "invalid '' secret data for 'ldap' STS provider: required fields username, password", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + err := ValidateSTSSecret(tt.provider, tt.secret) + if tt.err != "" { + assert.Error(t, err, tt.err) + } else { + assert.NilError(t, err) + } + }) + } +} + func bucketStub(bucket sourcev1.Bucket, endpoint string) *sourcev1.Bucket { b := bucket.DeepCopy() b.Spec.Endpoint = endpoint