From 0b53aae542b36714c7eaea6f51b3e6f99476b634 Mon Sep 17 00:00:00 2001 From: "lukasz.widera@vshn.ch" Date: Mon, 15 Jul 2024 11:14:07 +0200 Subject: [PATCH 1/5] add minio server's api call to set user's password it's important feature as we had a bug where password was lost/broken for some reason and caused operational issues in our cluster. --- operator/user/connector.go | 2 ++ operator/user/observe.go | 24 ++++++++++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/operator/user/connector.go b/operator/user/connector.go index 98ea26a..e20db28 100644 --- a/operator/user/connector.go +++ b/operator/user/connector.go @@ -27,6 +27,7 @@ type connector struct { type userClient struct { ma *madmin.AdminClient + kube client.Client recorder event.Recorder } @@ -56,6 +57,7 @@ func (c *connector) Connect(ctx context.Context, mg resource.Managed) (managed.E uc := &userClient{ ma: ma, + kube: c.kube, recorder: c.recorder, } diff --git a/operator/user/observe.go b/operator/user/observe.go index 14202ca..221503d 100644 --- a/operator/user/observe.go +++ b/operator/user/observe.go @@ -10,6 +10,8 @@ import ( "github.com/crossplane/crossplane-runtime/pkg/resource" "github.com/minio/madmin-go/v3" miniov1 "github.com/vshn/provider-minio/apis/minio/v1" + k8svi "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/types" ) const ( @@ -59,6 +61,28 @@ func (u *userClient) Observe(ctx context.Context, mg resource.Managed) (managed. user.SetConditions(miniov1.Disabled()) } + if mg.GetDeletionTimestamp() == nil { + + secret := k8svi.Secret{} + + err = u.kube.Get(ctx, types.NamespacedName{ + Namespace: mg.GetWriteConnectionSecretToReference().Namespace, + Name: mg.GetWriteConnectionSecretToReference().Name, + }, &secret) + if err != nil { + return managed.ExternalObservation{}, err + } + + // this here prevents painful user errors with password generation using bash shell and `echo` + // if You want to use `echo` to generate a password, use `echo -n` to prevent adding a newline + strippedFromNewline := strings.ReplaceAll(string(secret.Data[AccessKeyName]), "\n", "") + + err = u.ma.SetUser(ctx, string(secret.Data[AccessKeyName]), strippedFromNewline, madmin.AccountEnabled) + if err != nil { + return managed.ExternalObservation{}, err + } + } + return managed.ExternalObservation{ResourceExists: true, ResourceUpToDate: true}, nil } From 6514348c044011d7394f9324ddaf0404d3efc5d7 Mon Sep 17 00:00:00 2001 From: "lukasz.widera@vshn.ch" Date: Mon, 15 Jul 2024 11:57:43 +0200 Subject: [PATCH 2/5] removing stripping as it broke autogenerated passwords for some reason --- operator/user/observe.go | 6 +----- test/e2e/upload-object.sh | 8 +++++++- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/operator/user/observe.go b/operator/user/observe.go index 221503d..7aa9099 100644 --- a/operator/user/observe.go +++ b/operator/user/observe.go @@ -73,11 +73,7 @@ func (u *userClient) Observe(ctx context.Context, mg resource.Managed) (managed. return managed.ExternalObservation{}, err } - // this here prevents painful user errors with password generation using bash shell and `echo` - // if You want to use `echo` to generate a password, use `echo -n` to prevent adding a newline - strippedFromNewline := strings.ReplaceAll(string(secret.Data[AccessKeyName]), "\n", "") - - err = u.ma.SetUser(ctx, string(secret.Data[AccessKeyName]), strippedFromNewline, madmin.AccountEnabled) + err = u.ma.SetUser(ctx, string(secret.Data[AccessKeyName]), string(secret.Data[SecretKeyName]), madmin.AccountEnabled) if err != nil { return managed.ExternalObservation{}, err } diff --git a/test/e2e/upload-object.sh b/test/e2e/upload-object.sh index 77c9858..291733e 100755 --- a/test/e2e/upload-object.sh +++ b/test/e2e/upload-object.sh @@ -12,4 +12,10 @@ access_key=$(kubectl -n "${secret_namespace}" get secret "${secret_name}" -o jso secret_key=$(kubectl -n "${secret_namespace}" get secret "${secret_name}" -o jsonpath='{.data.AWS_SECRET_ACCESS_KEY}' | base64 -d) export MC_HOST_minio=http://${access_key}:${secret_key}@${endpoint} -"${GOBIN}/mc" cp --quiet "${file_path}" "minio/${bucket_name}" +echo "Uploading object to bucket: ${bucket_name}" +echo "File path: ${file_path}" +echo "Endpoint: ${endpoint}" +echo "Access key: ${access_key}" +echo "Secret key: ${secret_key}" + +"${GOBIN}/mc" cp --quiet --debug "${file_path}" "minio/${bucket_name}" From bd9531e8626bf52ed4d727b898efdec668f89399 Mon Sep 17 00:00:00 2001 From: "lukasz.widera@vshn.ch" Date: Thu, 18 Jul 2024 10:28:25 +0200 Subject: [PATCH 3/5] refactoring code --- .gitignore | 1 + kind/config.yaml | 6 +++--- operator/minioutil/admin.go | 16 +++++++++++++--- operator/user/connector.go | 22 ++++++++++++++++------ operator/user/observe.go | 19 +++++++++++++++++-- operator/user/update.go | 21 +++++++++++++++++++++ 6 files changed, 71 insertions(+), 14 deletions(-) diff --git a/.gitignore b/.gitignore index abe47e3..501a249 100644 --- a/.gitignore +++ b/.gitignore @@ -17,3 +17,4 @@ __debug* # work /.work/ +.vscode/ \ No newline at end of file diff --git a/kind/config.yaml b/kind/config.yaml index e84013b..22fffd1 100644 --- a/kind/config.yaml +++ b/kind/config.yaml @@ -10,12 +10,12 @@ nodes: node-labels: "ingress-ready=true" extraPortMappings: - containerPort: 80 - hostPort: 8088 + hostPort: 10088 protocol: TCP - containerPort: 443 - hostPort: 8443 + hostPort: 10443 protocol: TCP # registry can't be sensibly exposed via Ingress under 127.0.0.0.nip.io host with subpath - containerPort: 30500 - hostPort: 5000 + hostPort: 15000 protocol: TCP diff --git a/operator/minioutil/admin.go b/operator/minioutil/admin.go index e614a86..d832343 100644 --- a/operator/minioutil/admin.go +++ b/operator/minioutil/admin.go @@ -14,19 +14,29 @@ import ( // It can be used to assign a policy to a usser. func NewMinioAdmin(ctx context.Context, c client.Client, config *providerv1.ProviderConfig) (*madmin.AdminClient, error) { + secret, tls, parsed, err := ExtractDataFromProviderConfig(ctx, c, config) + if err != nil { + return nil, err + } + + return madmin.New(parsed.Host, string(secret.Data[MinioIDKey]), string(secret.Data[MinioSecretKey]), tls) +} + +// this is the helper function that is used in the NewMinioClient function +func ExtractDataFromProviderConfig(ctx context.Context, c client.Client, config *providerv1.ProviderConfig) (*corev1.Secret, bool, *url.URL, error) { secret := &corev1.Secret{} key := client.ObjectKey{Name: config.Spec.Credentials.APISecretRef.Name, Namespace: config.Spec.Credentials.APISecretRef.Namespace} err := c.Get(ctx, key, secret) if err != nil { - return nil, err + return nil, false, nil, err } parsed, err := url.Parse(config.Spec.MinioURL) if err != nil { - return nil, err + return nil, false, nil, err } tls := isTLSEnabled(parsed) - return madmin.New(parsed.Host, string(secret.Data[MinioIDKey]), string(secret.Data[MinioSecretKey]), tls) + return secret, tls, parsed, nil } diff --git a/operator/user/connector.go b/operator/user/connector.go index e20db28..6063947 100644 --- a/operator/user/connector.go +++ b/operator/user/connector.go @@ -3,6 +3,7 @@ package user import ( "context" "fmt" + "net/url" "github.com/crossplane/crossplane-runtime/pkg/event" "github.com/crossplane/crossplane-runtime/pkg/reconciler/managed" @@ -26,9 +27,11 @@ type connector struct { } type userClient struct { - ma *madmin.AdminClient - kube client.Client - recorder event.Recorder + ma *madmin.AdminClient + kube client.Client + recorder event.Recorder + url *url.URL + tlsSettings bool } func (c *connector) Connect(ctx context.Context, mg resource.Managed) (managed.ExternalClient, error) { @@ -55,10 +58,17 @@ func (c *connector) Connect(ctx context.Context, mg resource.Managed) (managed.E return nil, err } + _, tls, parsed, err := minioutil.ExtractDataFromProviderConfig(ctx, c.kube, config) + if err != nil { + return nil, err + } + uc := &userClient{ - ma: ma, - kube: c.kube, - recorder: c.recorder, + ma: ma, + kube: c.kube, + recorder: c.recorder, + url: parsed, + tlsSettings: tls, } return uc, nil diff --git a/operator/user/observe.go b/operator/user/observe.go index 7aa9099..d867f0c 100644 --- a/operator/user/observe.go +++ b/operator/user/observe.go @@ -9,9 +9,12 @@ import ( "github.com/crossplane/crossplane-runtime/pkg/reconciler/managed" "github.com/crossplane/crossplane-runtime/pkg/resource" "github.com/minio/madmin-go/v3" + "github.com/minio/minio-go/v7" + "github.com/minio/minio-go/v7/pkg/credentials" miniov1 "github.com/vshn/provider-minio/apis/minio/v1" k8svi "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/types" + ctrl "sigs.k8s.io/controller-runtime" ) const ( @@ -20,6 +23,7 @@ const ( ) func (u *userClient) Observe(ctx context.Context, mg resource.Managed) (managed.ExternalObservation, error) { + log := ctrl.LoggerFrom(ctx) user, ok := mg.(*miniov1.User) if !ok { @@ -73,10 +77,21 @@ func (u *userClient) Observe(ctx context.Context, mg resource.Managed) (managed. return managed.ExternalObservation{}, err } - err = u.ma.SetUser(ctx, string(secret.Data[AccessKeyName]), string(secret.Data[SecretKeyName]), madmin.AccountEnabled) + mclient, err := minio.New(u.url.Host, &minio.Options{ + Creds: credentials.NewStaticV4(string(secret.Data[AccessKeyName]), string(secret.Data[SecretKeyName]), ""), + Secure: u.tlsSettings, + }) if err != nil { - return managed.ExternalObservation{}, err + return managed.ExternalObservation{ResourceUpToDate: false, ResourceExists: true}, nil + } + + _, err = mclient.ListBuckets(context.Background()) + // AccessDenied is ok in this context, because we just want to check if the user has working credentials + if err != nil && err.Error() != "Access Denied." { + return managed.ExternalObservation{ResourceUpToDate: false, ResourceExists: true}, nil } + + log.Info("user client created, everything went fine " + string(secret.Data[AccessKeyName]) + " " + string(secret.Data[SecretKeyName])) } return managed.ExternalObservation{ResourceExists: true, ResourceUpToDate: true}, nil diff --git a/operator/user/update.go b/operator/user/update.go index 6e44d2d..acae81b 100644 --- a/operator/user/update.go +++ b/operator/user/update.go @@ -8,7 +8,10 @@ import ( "github.com/crossplane/crossplane-runtime/pkg/reconciler/managed" "github.com/crossplane/crossplane-runtime/pkg/resource" "github.com/minio/madmin-go/v3" + miniov1 "github.com/vshn/provider-minio/apis/minio/v1" + k8svi "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/types" controllerruntime "sigs.k8s.io/controller-runtime" ) @@ -48,6 +51,24 @@ func (u *userClient) Update(ctx context.Context, mg resource.Managed) (managed.E return managed.ExternalUpdate{}, err } + if mg.GetDeletionTimestamp() == nil { + + secret := k8svi.Secret{} + + err = u.kube.Get(ctx, types.NamespacedName{ + Namespace: mg.GetWriteConnectionSecretToReference().Namespace, + Name: mg.GetWriteConnectionSecretToReference().Name, + }, &secret) + if err != nil { + return managed.ExternalUpdate{}, err + } + + err = u.ma.SetUser(ctx, string(secret.Data[AccessKeyName]), string(secret.Data[SecretKeyName]), madmin.AccountEnabled) + if err != nil { + return managed.ExternalUpdate{}, err + } + } + u.emitUpdateEvent(user) return managed.ExternalUpdate{}, nil } From cb808f557511b7a68323b6e18d2cf716ccbd6ba5 Mon Sep 17 00:00:00 2001 From: "lukasz.widera@vshn.ch" Date: Thu, 18 Jul 2024 10:50:01 +0200 Subject: [PATCH 4/5] fixing ports in e2e tests --- Makefile.vars.mk | 2 +- test/local.mk | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Makefile.vars.mk b/Makefile.vars.mk index 5a97da0..201353a 100644 --- a/Makefile.vars.mk +++ b/Makefile.vars.mk @@ -21,7 +21,7 @@ GIT_TAG = $(shell git symbolic-ref -q --short HEAD || git describe --tags --exac IMG_TAG = $(subst /,_,$(GIT_TAG)) # Image URL to use all building/pushing image targets CONTAINER_IMG ?= $(CONTAINER_REGISTRY)/$(PROJECT_OWNER)/$(PROJECT_NAME)/controller:$(IMG_TAG) -LOCAL_PACKAGE_IMG = localhost:5000/$(PROJECT_OWNER)/$(PROJECT_NAME)/package:$(IMG_TAG) +LOCAL_PACKAGE_IMG = localhost:15000/$(PROJECT_OWNER)/$(PROJECT_NAME)/package:$(IMG_TAG) GHCR_PACKAGE_IMG ?= $(CONTAINER_REGISTRY)/$(PROJECT_OWNER)/$(PROJECT_NAME)/provider:$(IMG_TAG) UPBOUND_PACKAGE_IMG ?= $(UPBOUND_CONTAINER_REGISTRY)/$(PROJECT_OWNER)/$(PROJECT_NAME):$(IMG_TAG) diff --git a/test/local.mk b/test/local.mk index 332289c..aef53b1 100644 --- a/test/local.mk +++ b/test/local.mk @@ -10,7 +10,7 @@ INTEGRATION_TEST_DEBUG_OUTPUT ?= false .PHONY: local-install local-install: export KUBECONFIG = $(KIND_KUBECONFIG) # for ControllerConfig: -local-install: export INTERNAL_PACKAGE_IMG = registry.registry-system.svc.cluster.local:5000/$(PROJECT_OWNER)/$(PROJECT_NAME)/package:$(IMG_TAG) +local-install: export INTERNAL_PACKAGE_IMG = registry.registry-system.svc.cluster.local:15000/$(PROJECT_OWNER)/$(PROJECT_NAME)/package:$(IMG_TAG) local-install: kind-load-image crossplane-setup registry-setup .local-package-push minio-setup ## Install Operator in local cluster yq e '.spec.metadata.annotations."local.dev/installed"="$(shell date)"' test/controllerconfig-minio.yaml | kubectl apply -f - yq e '.spec.package=strenv(INTERNAL_PACKAGE_IMG)' test/provider-minio.yaml | kubectl apply -f - From f04fba409b0e81c7de31fd232080aff6c7dae4a6 Mon Sep 17 00:00:00 2001 From: "lukasz.widera@vshn.ch" Date: Thu, 18 Jul 2024 11:09:11 +0200 Subject: [PATCH 5/5] fixing e2e --- Makefile.vars.mk | 2 +- kind/config.yaml | 6 +++--- test/local.mk | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Makefile.vars.mk b/Makefile.vars.mk index 201353a..5a97da0 100644 --- a/Makefile.vars.mk +++ b/Makefile.vars.mk @@ -21,7 +21,7 @@ GIT_TAG = $(shell git symbolic-ref -q --short HEAD || git describe --tags --exac IMG_TAG = $(subst /,_,$(GIT_TAG)) # Image URL to use all building/pushing image targets CONTAINER_IMG ?= $(CONTAINER_REGISTRY)/$(PROJECT_OWNER)/$(PROJECT_NAME)/controller:$(IMG_TAG) -LOCAL_PACKAGE_IMG = localhost:15000/$(PROJECT_OWNER)/$(PROJECT_NAME)/package:$(IMG_TAG) +LOCAL_PACKAGE_IMG = localhost:5000/$(PROJECT_OWNER)/$(PROJECT_NAME)/package:$(IMG_TAG) GHCR_PACKAGE_IMG ?= $(CONTAINER_REGISTRY)/$(PROJECT_OWNER)/$(PROJECT_NAME)/provider:$(IMG_TAG) UPBOUND_PACKAGE_IMG ?= $(UPBOUND_CONTAINER_REGISTRY)/$(PROJECT_OWNER)/$(PROJECT_NAME):$(IMG_TAG) diff --git a/kind/config.yaml b/kind/config.yaml index 22fffd1..e84013b 100644 --- a/kind/config.yaml +++ b/kind/config.yaml @@ -10,12 +10,12 @@ nodes: node-labels: "ingress-ready=true" extraPortMappings: - containerPort: 80 - hostPort: 10088 + hostPort: 8088 protocol: TCP - containerPort: 443 - hostPort: 10443 + hostPort: 8443 protocol: TCP # registry can't be sensibly exposed via Ingress under 127.0.0.0.nip.io host with subpath - containerPort: 30500 - hostPort: 15000 + hostPort: 5000 protocol: TCP diff --git a/test/local.mk b/test/local.mk index aef53b1..332289c 100644 --- a/test/local.mk +++ b/test/local.mk @@ -10,7 +10,7 @@ INTEGRATION_TEST_DEBUG_OUTPUT ?= false .PHONY: local-install local-install: export KUBECONFIG = $(KIND_KUBECONFIG) # for ControllerConfig: -local-install: export INTERNAL_PACKAGE_IMG = registry.registry-system.svc.cluster.local:15000/$(PROJECT_OWNER)/$(PROJECT_NAME)/package:$(IMG_TAG) +local-install: export INTERNAL_PACKAGE_IMG = registry.registry-system.svc.cluster.local:5000/$(PROJECT_OWNER)/$(PROJECT_NAME)/package:$(IMG_TAG) local-install: kind-load-image crossplane-setup registry-setup .local-package-push minio-setup ## Install Operator in local cluster yq e '.spec.metadata.annotations."local.dev/installed"="$(shell date)"' test/controllerconfig-minio.yaml | kubectl apply -f - yq e '.spec.package=strenv(INTERNAL_PACKAGE_IMG)' test/provider-minio.yaml | kubectl apply -f -