[occm] [cinder-csi-plugin] Sanitize AZ name

[scrungus]

trafficstars

What this PR does / why we need it: AZ names can contain unicode characters, for example "Württemberg". The current behaviour is that this causes the occm pod to crash:

$ crictl logs a358b0388996d (openstack-cloud-controller-manager)
error syncing 'test-hshu2jf73dym-default-worker-6s4mq-s9swb': Node "test-hshu2jf73dym-default-worker-6s4mq-s9swb" is invalid: metadata.labels: Invalid value: "Württemberg Test": a valid label must be an empty string or consist of alphanumeric characters, '-', '_' or '.', and must start and end with an alphanumeric character (e.g. 'MyValue',  or 'my_value',  or '12345', regex used for validation is '(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])?'), requeuing

This PR sanitizes the label value before it is applied. We also see the same problem in cinder-csi-plugin:

$ kubectl logs -n openstack-system openstack-cinder-csi-nodeplugin-cl5tc
...
I0822 14:53:22.576401       1 main.go:101] Received NotifyRegistrationStatus call: &RegistrationStatus{PluginRegistered:false,Error:RegisterPlugin error -- plugin registration failed with err: error updating Node object with CSI driver node info: error updating node: timed out waiting for the condition; caused by: failed to patch status "{\"metadata\":{\"annotations\":{\"csi.volume.kubernetes.io/nodeid\":\"{\\\"cinder.csi.openstack.org\\\":\\\"0699b10e-b4bc-4129-9280-5b7c8556418f\\\",\\\"csi.tigera.io\\\":\\\"sst-yde-test-1-yt3243sf6os5-control-plane-m4hpg\\\"}\"},\"labels\":{\"topology.cinder.csi.openstack.org/zone\":\"Düdingen Test\"}}}" for node "sst-yde-test-1-yt3243sf6os5-control-plane-m4hpg": Node "sst-yde-test-1-yt3243sf6os5-control-plane-m4hpg" is invalid: metadata.labels: Invalid value: "Düdingen Test": a valid label must be an empty string or consist of alphanumeric characters, '-', '_' or '.', and must start and end with an alphanumeric character (e.g. 'MyValue',  or 'my_value',  or '12345', regex used for validation is '(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])?'),}

So this change also sanitises the label coming from the metadata service. Special notes for reviewers:

I believe a similar fix is needed for cinder-csi-plugin Release note:

Fixes a bug where unicode characters in the openstack availability zone can crash the OCCM or cinder-csi-plugin pods.

[linux-foundation-easycla[bot]]

The committers listed above are authorized under a signed CLA.

• :white_check_mark: login: scrungus / name: scrungus (b9cdc6c23310e732189b03a9c754c32f85cfa455)

[k8s-ci-robot]

Welcome @scrungus!

It looks like this is your first PR to kubernetes/cloud-provider-openstack 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes/cloud-provider-openstack has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. :smiley:

[k8s-ci-robot]

Hi @scrungus. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[scrungus]

I am unsure as to why the availability zone information comes from gophercloud in the occm case, and the metadata service in the cinder-csi-plugin. It may make sense to get occm to use the metadata service too, unless there's a specific reason otherwise.

[dulek]

/ok-to-test

[dulek]

I am unsure as to why the availability zone information comes from gophercloud in the occm case, and the metadata service in the cinder-csi-plugin. It may make sense to get occm to use the metadata service too, unless there's a specific reason otherwise.

Doesn't cinder-csi-plugin run on every node, when occm can run on any node of the cluster?

[jichenjc]

Doesn't cinder-csi-plugin run on every node, when occm can run on any node of the cluster?

yes, it is CSI need run every node, as it has control and data plane both

[jichenjc]

/test openstack-cloud-controller-manager-ovn-e2e-test

[jichenjc]

The service has some problem in start up ..

>>>>>>> FAIL: Timeout when waiting for the IP address 172.24.5.107(VIP) accessible

but not sure it's related to this PR, not sure worthy to try an empty PR to see whether it's something else cause the problem

[jichenjc]

/retest

[scrungus]

Doesn't cinder-csi-plugin run on every node, when occm can run on any node of the cluster?

yes, it is CSI need run every node, as it has control and data plane both

I see, that makes sense then.

[jichenjc]

/lgtm

[scrungus]

@jichenjc we have a client that we did this work for, they would like to be included in a Co-Authored-By in the commit message. Could you please wait for me to update this before merging?

[jichenjc]

I hold this, you can feel free to unhold when you are ready

/hold

[scrungus]

ready to merge

@jichenjc you might need to reapprove this as I force-pushed.

[jichenjc]

/approve /lgtm /hold cancel

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jichenjc

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• ~~OWNERS~~ [jichenjc]

Approvers can indicate their approval by writing /approve in a comment Approvers can cancel approval by writing /approve cancel in a comment