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

DC names are not properly sanitized #1141

Closed
olim7t opened this issue Dec 18, 2023 · 0 comments · Fixed by #1246
Closed

DC names are not properly sanitized #1141

olim7t opened this issue Dec 18, 2023 · 0 comments · Fixed by #1246
Assignees
Labels
bug Something isn't working done Issues in the state 'done' naming

Comments

@olim7t
Copy link
Contributor

olim7t commented Dec 18, 2023

If a cluster declares a datacenter with invalid characters:

apiVersion: k8ssandra.io/v1alpha1
kind: K8ssandraCluster
metadata:
  namespace: k8ssandra-operator
  name: demo
spec:
  cassandra:
    serverVersion: 4.1.0
    datacenters:
      - metadata:
          name: dc_1   # <<< invalid Kubernetes name
        size: 1
        racks:
          - name: rack1
        storageConfig:
          cassandraDataVolumeClaimSpec:
            storageClassName: standard
            accessModes:
              - ReadWriteOnce
            resources:
              requests:
                storage: 100Mi

The name is not properly sanitized, which leads to this error:

ConfigMap "demo-dc_1-metrics-agent-config" is invalid: metadata.name: Invalid value: "demo-dc_1-metrics-agent-config": a lowercase RFC 1123 subdomain must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character (e.g. 'example.com', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*')

The problem is not limited to that ConfigMap: even after fixing it (here), the invalid name causes another error when the operator tries to create the CassandraDatacenter.

We should either reject invalid DC names in the validation webhook, or sanitize them and maybe use the original value as the datacenterName.

@olim7t olim7t added the bug Something isn't working label Dec 18, 2023
@adejanovski adejanovski added the ready Issues in the state 'ready' label Feb 15, 2024
@olim7t olim7t self-assigned this Mar 15, 2024
@adejanovski adejanovski added in-progress Issues in the state 'in-progress' and removed ready Issues in the state 'ready' labels Mar 15, 2024
olim7t added a commit to olim7t/k8ssandra-operator that referenced this issue Mar 15, 2024
@olim7t olim7t mentioned this issue Mar 16, 2024
5 tasks
@adejanovski adejanovski added ready-for-review Issues in the state 'ready-for-review' in-progress Issues in the state 'in-progress' and removed in-progress Issues in the state 'in-progress' ready-for-review Issues in the state 'ready-for-review' labels Mar 16, 2024
olim7t added a commit to olim7t/k8ssandra-operator that referenced this issue Mar 18, 2024
- reject cluster creation if a DC has an invalid name
- always use sanitized DC name override when naming secondary resources
- use cluster name override for metrics agent configMap

Fixes k8ssandra#1138
Fixes k8ssandra#1141
@adejanovski adejanovski added ready-for-review Issues in the state 'ready-for-review' review Issues in the state 'review' and removed in-progress Issues in the state 'in-progress' ready-for-review Issues in the state 'ready-for-review' labels Mar 18, 2024
olim7t added a commit that referenced this issue Mar 19, 2024
- reject cluster creation if a DC has an invalid name
- always use sanitized DC name override when naming secondary resources
- use cluster name override for metrics agent configMap

Fixes #1138
Fixes #1141
@adejanovski adejanovski added done Issues in the state 'done' and removed review Issues in the state 'review' labels Mar 19, 2024
@olim7t olim7t added the naming label Mar 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working done Issues in the state 'done' naming
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants