cockroachdb / cockroachdb/cockroach-operator

[BUG] Updating the field `spec.ingress.sql.tls.secretName` is not reflected in the sql ingress object

Open
#920 3 comments 0 reactions 0 assignees View on GitHub
Dominant language
Go
Stars
318
Forks
104
Avg merge
1d 6h
Merged PRs (30d)
1

Description

**What version of operator are you using?**
commit 561cf47d783c368fd8795acb82a5a39099a35984 (HEAD -> master)

**What operating system and processor architecture are you using (`kubectl version`)?**
Ubuntu. 20.04

kubectl version Output

$ kubectl version

Client Version: version.Info{Major:"1", Minor:"24", GitVersion:"v1.24.0", GitCommit:"4ce5a8954017644c5420bae81d72b09b735c21f0", GitTreeState:"clean", BuildDate:"2022-05-03T13:46:05Z", GoVersion:"go1.18.1", Compiler:"gc", Platform:"darwin/amd64"}
Kustomize Version: v4.5.4
Server Version: version.Info{Major:"1", Minor:"22", GitVersion:"v1.22.9", GitCommit:"6df4433e288edc9c40c2e344eb336f63fad45cd2", GitTreeState:"clean", BuildDate:"2022-05-19T19:53:08Z", GoVersion:"go1.16.15", Compiler:"gc", Platform:"linux/amd64"}
WARNING: version difference between client (1.24) and server (1.22) exceeds the supported minor version skew of +/-1

**What did you do?**

We found that when we updated the field `spec.ingress.sql.tls.secretName` in the CR, the change is not reflected in the sql ingress resource. The field `spec.ingress.sql.tls.secretName` is only effective when initially creating the sql ingress resource.

### Reproduce

1. We first applied the operator yaml file and crd yaml file to deploy the operator and used the following cr.yaml file to deploy the cockroachdb cluster by using `kubectl apply -f cr.yaml -n cockroach-operator-system`
**cr.yaml:**
```yaml
apiVersion: crdb.cockroachlabs.com/v1alpha1
kind: CrdbCluster
metadata:
name: test-cluster
spec:
additionalLabels:
crdb: is-cool
dataStore:
pvc:
spec:
accessModes:
- ReadWriteOnce
resources:
requests:
storage: 1Gi
volumeMode: Filesystem
image:
name: cockroachdb/cockroach:v21.2.10
ingress:
sql:
host: MyHost
tls:
- secretName: MySecretName
nodes: 3
resources:
limits:
cpu: 2
memory: 2Gi
requests:
cpu: 100m
memory: 1Gi
tlsEnabled: true
```

2. Then we updated the field `spec.ingress.sql.tls.secretName` by deploying the following yaml file:

```yaml
apiVersion: crdb.cockroachlabs.com/v1alpha1
kind: CrdbCluster
metadata:
name: test-cluster
spec:
additionalLabels:
crdb: is-cool
dataStore:
pvc:
spec:
accessModes:
- ReadWriteOnce
resources:
requests:
storage: 1Gi
volumeMode: Filesystem
image:
name: cockroachdb/cockroach:v21.2.10
ingress:
sql:
host: MyHost
tls:
- secretName: MyAnotherSecretName
nodes: 3
resources:
limits:
cpu: 2
memory: 2Gi
requests:
cpu: 100m
memory: 1Gi
tlsEnabled: true
```

**What did you see?**

`spec.tls.0.secret_name` under `ingress` was not updated in the cluster. We expected that `spec.tls.0.secret_name` under `ingress` would be changed from `MySecretName` to `MyAnotherSecretName`, but there was no such change.

**Possible root cause**
We searched through cockroach operator code based and it seemed that the cockroach operator does not have code update `spec.ingress.sql.tls.secretName`. So no matter what new value is assigned to `spec.ingress.sql.tls.secretName`, nothing will be reflected in the cluster.

There is a similar feature in the `spec.ingress.ui`, and it is implemented [here](https://github.com/cockroachdb/cockroach-operator/blob/dda0a6308b0f1c64d41e0851a5ef8cb60cd98a37/pkg/resource/ui_ingress.go#L134)

This is a bug, since `secretName` cannot be updated and there were no messages indicating that our input got rejected.

**Possible fix**
We also ran an experiment after adding these lines to `pkg > resource > sql_ingress.go > BuildV1Ingress` at [line 92](https://github.com/cockroachdb/cockroach-operator/blob/dda0a6308b0f1c64d41e0851a5ef8cb60cd98a37/pkg/resource/sql_ingress.go#L92) (right before the function 's final `return nil`):
```go
for i := range ingressConfig.SQL.TLS {
ingress.Spec.TLS = append(ingress.Spec.TLS, v1.IngressTLS{
Hosts: ingressConfig.SQL.TLS[i].Hosts,
SecretName: ingressConfig.SQL.TLS[i].SecretName,
})
}
```

And after remaking the image and used the image we made to deploy the operator, the change in `spec.ingress.sql.tls.secretName` showed up in the cluster.

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.