Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Regression in 5.0.8 from #1716 when using operator-ca-tls #1766

Closed
JakobPetersson opened this issue Sep 11, 2023 · 3 comments
Closed

Regression in 5.0.8 from #1716 when using operator-ca-tls #1766

JakobPetersson opened this issue Sep 11, 2023 · 3 comments
Assignees

Comments

@JakobPetersson
Copy link

There appears to be two related regressions in 5.0.8 introduced by #1716.

First issue:
Uninitialized map certsData leading to "assignment to entry in nil map".

var certsData map[string][]byte

Second issue:
The changes done in #1716 does not appear to align with other parts of the minio-operator about what keys the operator-ca-tls secret should have. Since #1716 it appears that the secret operator-ca-tls is required to contain all three public.crt, tls.crt and ca.crt. In the documentation it is stated that operator-ca-tls should only contain ca.crt when using a certificate manager such as cert-manager.
https://github.com/minio/operator/blob/master/docs/tls.md#using-your-own-ca-certificate-for-minio-operator
https://github.com/minio/operator/blob/master/docs/cert-manager.md#create-operator-ca-tls-secret

Expected Behavior

First issue:
The function checkOperatorCaForTenant should not cause a panic due to unassigned map.

Second issue:
The implementation should support all the documented values used in operator-ca-tls secret.
Including only setting ca.crt as documented for certificate managers such as cert-manager.

Current Behavior

First issue:
The operator panics when assigning to entry in certsData on line 151:

certsData[common.PublicCRT] = operatorPublicCert

Show full log I0911 12:17:17.578256 1 controller.go:71] Starting MinIO Operator I0911 12:17:17.656453 1 main-controller.go:278] Setting up event handlers I0911 12:17:17.727432 1 main-controller.go:502] Using Kubernetes CSR Version: v1 I0911 12:17:17.727564 1 main-controller.go:522] STS Api server is not enabled, not starting I0911 12:17:17.727798 1 leaderelection.go:248] attempting to acquire leader lease minio-operator/minio-operator-lock... I0911 12:17:17.738686 1 leaderelection.go:258] successfully acquired lease minio-operator/minio-operator-lock I0911 12:17:17.738785 1 main-controller.go:551] minio-operator-77894c7fc5-qx9m8: I am the leader, applying leader labels on myself I0911 12:17:17.738822 1 main-controller.go:397] Waiting for Upgrade Server to start I0911 12:17:17.738844 1 main-controller.go:401] Starting Tenant controller I0911 12:17:17.738849 1 main-controller.go:404] Waiting for informer caches to sync I0911 12:17:17.738889 1 main-controller.go:353] Starting HTTP Upgrade Tenant Image server I0911 12:17:17.839103 1 main-controller.go:409] Starting workers I0911 12:17:17.839143 1 main-controller.go:442] Console TLS is not enabled I0911 12:17:17.956558 1 status.go:89] Hit conflict issue, getting latest version of tenant E0911 12:17:22.759053 1 runtime.go:79] Observed a panic: "assignment to entry in nil map" (assignment to entry in nil map) goroutine 578 [running]: k8s.io/apimachinery/pkg/util/runtime.logPanic({0x223cca0?, 0x4171b20}) k8s.io/apimachinery@v0.26.1/pkg/util/runtime/runtime.go:75 +0x85 k8s.io/apimachinery/pkg/util/runtime.HandleCrash({0x0, 0x0, 0xc00095f4e0?}) k8s.io/apimachinery@v0.26.1/pkg/util/runtime/runtime.go:49 +0x6b panic({0x223cca0?, 0x4171b20?}) runtime/panic.go:914 +0x21f github.com/minio/operator/pkg/controller.(*Controller).checkOperatorCaForTenant(0xc000272f00, {0x419c3e8, 0x5553140}, 0xc000d47800) github.com/minio/operator/pkg/controller/minio.go:151 +0x1bf github.com/minio/operator/pkg/controller.(*Controller).syncHandler(0xc000272f00, {0xc000c8b410, 0x21}) github.com/minio/operator/pkg/controller/main-controller.go:955 +0x1cca github.com/minio/operator/pkg/controller.(*Controller).processNextWorkItem.func1({0x20ea2c0?, 0xc00095f4e0}) github.com/minio/operator/pkg/controller/main-controller.go:673 +0x285 github.com/minio/operator/pkg/controller.(*Controller).processNextWorkItem(0xc000272f00) github.com/minio/operator/pkg/controller/main-controller.go:696 +0x5a github.com/minio/operator/pkg/controller.(*Controller).runWorker(0xc00099c6a0?) github.com/minio/operator/pkg/controller/main-controller.go:626 +0x3c k8s.io/apimachinery/pkg/util/wait.BackoffUntil.func1(0x30?) k8s.io/apimachinery@v0.26.1/pkg/util/wait/wait.go:157 +0x33 k8s.io/apimachinery/pkg/util/wait.BackoffUntil(0xc00099e000?, {0x41789a0, 0xc000f88ff0}, 0x1, 0xc000114960) k8s.io/apimachinery@v0.26.1/pkg/util/wait/wait.go:158 +0xaf k8s.io/apimachinery/pkg/util/wait.JitterUntil(0xc00099e000?, 0x3b9aca00, 0x0, 0x80?, 0xc00099c710?) k8s.io/apimachinery@v0.26.1/pkg/util/wait/wait.go:135 +0x7f k8s.io/apimachinery/pkg/util/wait.Until(0xc00099c7d0?, 0x932305?, 0xc00015b780?) k8s.io/apimachinery@v0.26.1/pkg/util/wait/wait.go:92 +0x1e created by github.com/minio/operator/pkg/controller.leaderRun in goroutine 532 github.com/minio/operator/pkg/controller/main-controller.go:412 +0x25a E0911 12:17:22.759125 1 runtime.go:79] Observed a panic: "assignment to entry in nil map" (assignment to entry in nil map) goroutine 578 [running]: k8s.io/apimachinery/pkg/util/runtime.logPanic({0x223cca0?, 0x4171b20}) k8s.io/apimachinery@v0.26.1/pkg/util/runtime/runtime.go:75 +0x85 k8s.io/apimachinery/pkg/util/runtime.HandleCrash({0x0, 0x0, 0xc000000002?}) k8s.io/apimachinery@v0.26.1/pkg/util/runtime/runtime.go:49 +0x6b panic({0x223cca0?, 0x4171b20?}) runtime/panic.go:914 +0x21f k8s.io/apimachinery/pkg/util/runtime.HandleCrash({0x0, 0x0, 0xc00095f4e0?}) k8s.io/apimachinery@v0.26.1/pkg/util/runtime/runtime.go:56 +0xcd panic({0x223cca0?, 0x4171b20?}) runtime/panic.go:914 +0x21f github.com/minio/operator/pkg/controller.(*Controller).checkOperatorCaForTenant(0xc000272f00, {0x419c3e8, 0x5553140}, 0xc000d47800) github.com/minio/operator/pkg/controller/minio.go:151 +0x1bf github.com/minio/operator/pkg/controller.(*Controller).syncHandler(0xc000272f00, {0xc000c8b410, 0x21}) github.com/minio/operator/pkg/controller/main-controller.go:955 +0x1cca github.com/minio/operator/pkg/controller.(*Controller).processNextWorkItem.func1({0x20ea2c0?, 0xc00095f4e0}) github.com/minio/operator/pkg/controller/main-controller.go:673 +0x285 github.com/minio/operator/pkg/controller.(*Controller).processNextWorkItem(0xc000272f00) github.com/minio/operator/pkg/controller/main-controller.go:696 +0x5a github.com/minio/operator/pkg/controller.(*Controller).runWorker(0xc00099c6a0?) github.com/minio/operator/pkg/controller/main-controller.go:626 +0x3c k8s.io/apimachinery/pkg/util/wait.BackoffUntil.func1(0x30?) k8s.io/apimachinery@v0.26.1/pkg/util/wait/wait.go:157 +0x33 k8s.io/apimachinery/pkg/util/wait.BackoffUntil(0xc00099e000?, {0x41789a0, 0xc000f88ff0}, 0x1, 0xc000114960) k8s.io/apimachinery@v0.26.1/pkg/util/wait/wait.go:158 +0xaf k8s.io/apimachinery/pkg/util/wait.JitterUntil(0xc00099e000?, 0x3b9aca00, 0x0, 0x80?, 0xc00099c710?) k8s.io/apimachinery@v0.26.1/pkg/util/wait/wait.go:135 +0x7f k8s.io/apimachinery/pkg/util/wait.Until(0xc00099c7d0?, 0x932305?, 0xc00015b780?) k8s.io/apimachinery@v0.26.1/pkg/util/wait/wait.go:92 +0x1e created by github.com/minio/operator/pkg/controller.leaderRun in goroutine 532 github.com/minio/operator/pkg/controller/main-controller.go:412 +0x25a panic: assignment to entry in nil map [recovered] panic: assignment to entry in nil map [recovered] panic: assignment to entry in nil map

goroutine 578 [running]:
k8s.io/apimachinery/pkg/util/runtime.HandleCrash({0x0, 0x0, 0xc000000002?})
k8s.io/apimachinery@v0.26.1/pkg/util/runtime/runtime.go:56 +0xcd
panic({0x223cca0?, 0x4171b20?})
runtime/panic.go:914 +0x21f
k8s.io/apimachinery/pkg/util/runtime.HandleCrash({0x0, 0x0, 0xc00095f4e0?})
k8s.io/apimachinery@v0.26.1/pkg/util/runtime/runtime.go:56 +0xcd
panic({0x223cca0?, 0x4171b20?})
runtime/panic.go:914 +0x21f
github.com/minio/operator/pkg/controller.(*Controller).checkOperatorCaForTenant(0xc000272f00, {0x419c3e8, 0x5553140}, 0xc000d47800)
github.com/minio/operator/pkg/controller/minio.go:151 +0x1bf
github.com/minio/operator/pkg/controller.(*Controller).syncHandler(0xc000272f00, {0xc000c8b410, 0x21})
github.com/minio/operator/pkg/controller/main-controller.go:955 +0x1cca
github.com/minio/operator/pkg/controller.(*Controller).processNextWorkItem.func1({0x20ea2c0?, 0xc00095f4e0})
github.com/minio/operator/pkg/controller/main-controller.go:673 +0x285
github.com/minio/operator/pkg/controller.(*Controller).processNextWorkItem(0xc000272f00)
github.com/minio/operator/pkg/controller/main-controller.go:696 +0x5a
github.com/minio/operator/pkg/controller.(*Controller).runWorker(0xc00099c6a0?)
github.com/minio/operator/pkg/controller/main-controller.go:626 +0x3c
k8s.io/apimachinery/pkg/util/wait.BackoffUntil.func1(0x30?)
k8s.io/apimachinery@v0.26.1/pkg/util/wait/wait.go:157 +0x33
k8s.io/apimachinery/pkg/util/wait.BackoffUntil(0xc00099e000?, {0x41789a0, 0xc000f88ff0}, 0x1, 0xc000114960)
k8s.io/apimachinery@v0.26.1/pkg/util/wait/wait.go:158 +0xaf
k8s.io/apimachinery/pkg/util/wait.JitterUntil(0xc00099e000?, 0x3b9aca00, 0x0, 0x80?, 0xc00099c710?)
k8s.io/apimachinery@v0.26.1/pkg/util/wait/wait.go:135 +0x7f
k8s.io/apimachinery/pkg/util/wait.Until(0xc00099c7d0?, 0x932305?, 0xc00015b780?)
k8s.io/apimachinery@v0.26.1/pkg/util/wait/wait.go:92 +0x1e
created by github.com/minio/operator/pkg/controller.leaderRun in goroutine 532
github.com/minio/operator/pkg/controller/main-controller.go:412 +0x25a

Second issue:
The operator fails to sync due to missing public.crt key in operator-ca-tls.

Show full log I0911 08:52:35.962578 1 controller.go:71] Starting MinIO Operator I0911 08:52:36.000111 1 main-controller.go:278] Setting up event handlers I0911 08:52:36.037013 1 main-controller.go:502] Using Kubernetes CSR Version: v1 I0911 08:52:36.037060 1 main-controller.go:522] STS Api server is not enabled, not starting I0911 08:52:36.037109 1 leaderelection.go:248] attempting to acquire leader lease minio-operator/minio-operator-lock... I0911 08:52:36.040591 1 main-controller.go:569] minio-operator-6ff54b8dc8-42thm: is the leader, removing any leader labels that I 'minio-operator-78cd5dc95f-7rbp5' might have I0911 08:52:41.303372 1 leaderelection.go:258] successfully acquired lease minio-operator/minio-operator-lock I0911 08:52:41.303471 1 main-controller.go:551] minio-operator-78cd5dc95f-7rbp5: I am the leader, applying leader labels on myself I0911 08:52:41.303503 1 main-controller.go:397] Waiting for Upgrade Server to start I0911 08:52:41.303536 1 main-controller.go:401] Starting Tenant controller I0911 08:52:41.303541 1 main-controller.go:404] Waiting for informer caches to sync I0911 08:52:41.303552 1 main-controller.go:409] Starting workers I0911 08:52:41.303562 1 main-controller.go:442] Console TLS is not enabled I0911 08:52:41.303649 1 main-controller.go:353] Starting HTTP Upgrade Tenant Image server I0911 08:52:41.409555 1 status.go:89] Hit conflict issue, getting latest version of tenant I0911 08:52:41.719036 1 status.go:89] Hit conflict issue, getting latest version of tenant E0911 08:52:41.908393 1 main-controller.go:697] error syncing 'my-ns/my-project-minio': missing 'public.crt' in minio-operator/operator-ca-tls secret E0911 08:53:01.460190 1 main-controller.go:697] error syncing 'my-ns/my-project-minio': missing 'public.crt' in minio-operator/operator-ca-tls secret E0911 08:53:06.503587 1 main-controller.go:697] error syncing 'my-ns/my-project-minio': missing 'public.crt' in minio-operator/operator-ca-tls secret I0911 08:53:18.981450 1 monitoring.go:124] 'my-ns/my-project-minio' Failed to get cluster health: Get "https://minio.gitlab-runner.svc.cluster.local/minio/health/cluster": dial tcp 172.30.107.223:443: connect: connection refused I0911 08:53:19.027758 1 monitoring.go:124] 'my-ns/my-project-minio' Failed to get cluster health: Get "https://minio.gitlab-runner.svc.cluster.local/minio/health/cluster": dial tcp 172.30.107.223:443: connect: connection refused E0911 08:53:24.127915 1 main-controller.go:697] error syncing 'my-ns/my-project-minio': missing 'public.crt' in minio-operator/operator-ca-tls secret I0911 08:53:35.333084 1 monitoring.go:124] 'my-ns/my-project-minio' Failed to get cluster health: Get "https://minio.gitlab-runner.svc.cluster.local/minio/health/cluster": dial tcp 172.30.107.223:443: i/o timeout I0911 08:53:36.351852 1 monitoring.go:124] 'my-ns/my-project-minio' Failed to get cluster health: Get "https://minio.gitlab-runner.svc.cluster.local/minio/health/cluster": dial tcp 172.30.107.223:443: connect: connection refused E0911 08:54:24.188604 1 main-controller.go:697] error syncing 'my-ns/my-project-minio': missing 'public.crt' in minio-operator/operator-ca-tls secret E0911 08:54:29.227144 1 main-controller.go:697] error syncing 'my-ns/my-project-minio': missing 'public.crt' in minio-operator/operator-ca-tls secret E0911 08:54:49.295500 1 main-controller.go:697] error syncing 'my-ns/my-project-minio': missing 'public.crt' in minio-operator/operator-ca-tls secret

Steps to Reproduce (for bugs)

First issue:

  1. Configure a secret named operator-ca-tls with public.crt set.

Second issue:

  1. Configure a secret named operator-ca-tls with only ca.crt set.

Regression

5.0.8

@qerub
Copy link

qerub commented Sep 11, 2023

Since #1716 it appears that the secret operator-ca-tls is required to contain all three public.crt, tls.crt and ca.crt.

I'm not sure this is true having the code in front of me:

operatorPublicCert, err := getOperatorCertFromSecret(operatorCaSecret.Data, common.PublicCRT)
if err != nil {
// If no public.crt is present we error, other certs are optional
return false, err
}
certsData[common.PublicCRT] = operatorPublicCert
operatorTLSCert, err := getOperatorCertFromSecret(operatorCaSecret.Data, common.TLSCRT)
if err == nil {
certsData[common.TLSCRT] = operatorTLSCert
}
operatorCACert, err := getOperatorCertFromSecret(operatorCaSecret.Data, common.CACRT)
if err == nil {
certsData[common.CACRT] = operatorCACert
}

I think public.crt is required (the first check) and the others are optional (as noted with the comment "other certs are optional"). It's still an regression though since it used to work with only ca.crt and now doesn't.

@harshavardhana
Copy link
Member

This PR would #1767 fix this.

@harshavardhana
Copy link
Member

fixed by #1767

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants