fix: correctly validate root credentials from secret

[twelho]

When using the MinIO operator to deploy a tenant, the operator-deployed sidecar container keeps crashing on startup:

2024/03/31 22:02:42 sidecar.go:48: Starting Sidecar
2024/03/31 22:03:28 sidecar.go:171: Missing root credentials in the configuration.
2024/03/31 22:03:28 sidecar.go:172: MinIO won't start

This occurs when deploying https://github.com/minio/operator/tree/master/helm/tenant, even with default values, with a configuration secret in the same form as the automatically generated one:

apiVersion: v1
kind: Secret
metadata:
  name: myminio-env-configuration
stringData:
  config.env: |-
    export MINIO_ROOT_USER="minio"
    export MINIO_ROOT_PASSWORD="minio123"
type: Opaque

After a bunch of messing around with the configuration I figured out that this is a bug introduced in #1437. Regardless of the configuration provided in the secret, the following check will always fail due to rootUserMissing := true, and even if that is fixed, it will still fail unless all four of the supported environment variables are defined, even though AFAICT they are supposed to be either-or (user/pass or access/secret):

operator/pkg/sidecar/sidecar.go

Lines 153 to 174 in a6a3e21

	// validate root creds in string
	rootUserMissing := true
	rootPassMissing := false
	dataStr := string(data)
	if !strings.Contains(dataStr, "MINIO_ROOT_USER") {
	rootUserMissing = true
	}
	if !strings.Contains(dataStr, "MINIO_ACCESS_KEY") {
	rootUserMissing = true
	}
	if !strings.Contains(dataStr, "MINIO_ROOT_PASSWORD") {
	rootPassMissing = true
	}
	if !strings.Contains(dataStr, "MINIO_SECRET_KEY") {
	rootPassMissing = true
	}
	if rootUserMissing || rootPassMissing {
	log.Println("Missing root credentials in the configuration.")
	log.Println("MinIO won't start")
	os.Exit(1)
	}

This PR fixes the issue by copying the intended logic from validator.go:

operator/pkg/validator/validator.go

Lines 110 to 128 in a6a3e21

	rootUserFound := false
	rootPwdFound := false
	scanner := bufio.NewScanner(file)
	newFile := ""
	for scanner.Scan() {
	line := scanner.Text()
	if strings.Contains(line, "MINIO_ROOT_USER") {
	rootUserFound = true
	}
	if strings.Contains(line, "MINIO_ACCESS_KEY") {
	rootUserFound = true
	}
	if strings.Contains(line, "MINIO_ROOT_PASSWORD") {
	rootPwdFound = true
	}
	if strings.Contains(line, "MINIO_SECRET_KEY") {
	rootPwdFound = true
	}

Tested working in a bare-metal deployment.

Two questions remain:

• How has this not been caught for over a year at this point? I'm beginning to doubt whether the operator is production-ready. Am I doing something incorrectly?
• There are three identical instances of the Missing root credentials in the configuration. termination in the code without a way to easily distinguish between them from stdout. Maybe a proper logging framework could be of use here?

[cniackz]

I cannot reproduce this issue and never seen it before. All I see is this log from sidecar:

2024/04/20 03:27:02 sidecar.go:48: Starting Sidecar

How exactly are you deploying it?

[cniackz]

Please look at Helm Steps for latest version:

https://github.com/minio/operator/wiki/Deploy-Operator-with-Helm

1. Download:

• https://github.com/minio/operator/blob/master/helm-releases/operator-5.0.14.tgz
• https://github.com/minio/operator/blob/master/helm-releases/tenant-5.0.14.tgz

2. Install:

helm install \
     --namespace minio-operator \
     --create-namespace \
     minio-operator ./operator-5.0.14.tgz

helm install \
  --namespace tenant-ns \
  --create-namespace \
  tenant-ns ./tenant-5.0.14.tgz

3. Check pods and logs:

$ k get pods -n tenant-ns
NAME               READY   STATUS    RESTARTS   AGE
myminio-pool-0-0   2/2     Running   0          35s
myminio-pool-0-1   2/2     Running   0          35s
myminio-pool-0-2   2/2     Running   0          35s
myminio-pool-0-3   2/2     Running   0          35s

[cniackz]

Hey @twelho, if your problem persists, could you please open an issue? Be sure to include all the steps required to reproduce it in a very detailed manner. Thank you for your time and interest in looking into this.

[ramondeklein]

@cniackz I am also able to spin up tenants, but this code looks wrong:

operator/pkg/sidecar/sidecar.go

Lines 152 to 174 in 01c56a5

	data := newSecret.Data["config.env"]
	// validate root creds in string
	rootUserMissing := true
	rootPassMissing := false
	dataStr := string(data)
	if !strings.Contains(dataStr, "MINIO_ROOT_USER") {
	rootUserMissing = true
	}
	if !strings.Contains(dataStr, "MINIO_ACCESS_KEY") {
	rootUserMissing = true
	}
	if !strings.Contains(dataStr, "MINIO_ROOT_PASSWORD") {
	rootPassMissing = true
	}
	if !strings.Contains(dataStr, "MINIO_SECRET_KEY") {
	rootPassMissing = true
	}
	if rootUserMissing || rootPassMissing {
	log.Println("Missing root credentials in the configuration.")
	log.Println("MinIO won't start")
	os.Exit(1)
	}

The variable rootUserMissing is always true and will always trigger the error message.

[pjuarezd]

Fixed on #2134, thanks @twelho

[twelho]

I will just say that copying my diff verbatim into c92f9e3 without citing the source is open source malpractice. I can gladly rebase PRs on request, and I believe you can also directly interact with my feature branch if you want to do it on your own. Another quick option is to just cherry-pick the original commit and then rename the file if you want to get going right away. I'll let this one slide, but just note that there are people who might not be too happy with the kind of steps you've taken here in general.

[pjuarezd]

you are cited and credited in PR description #2134

[twelho]

you are cited and credited in PR description #2134

And that is appreciated. However, what counts are the commits (i.e., their authors and descriptions), since PR metadata is not tied to version control. Referencing a user in a PR description/comment does not register them as a contributor on GH or anywhere else. Like said, no offense from my side, this is just to let you know for the future.