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

Fix: label values not validated (#355) #339

Merged
merged 3 commits into from
Jun 14, 2022

Conversation

jeffbanks
Copy link
Contributor

@jeffbanks jeffbanks commented Jun 3, 2022

What this PR does:
Provides cleaning of the cluster name being used for part of overall label value. The cluster name can contain illegal characters not allowed for label values.

Which issue(s) this PR fixes:
Fixes #335

Checklist

  • Changes manually tested
  • Automated Tests added/updated
  • Documentation added/updated
  • CHANGELOG.md updated (not required for documentation PRs)
  • CLA Signed: DataStax CLA

@jeffbanks jeffbanks requested a review from a team as a code owner June 3, 2022 19:08
@jeffbanks jeffbanks self-assigned this Jun 3, 2022
@jeffbanks jeffbanks added the bug Something isn't working label Jun 3, 2022
@jeffbanks jeffbanks requested a review from burmanm June 3, 2022 19:29
burmanm
burmanm previously requested changes Jun 5, 2022
pkg/oplabels/labels.go Show resolved Hide resolved
pkg/oplabels/labels.go Outdated Show resolved Hide resolved
pkg/oplabels/labels.go Outdated Show resolved Hide resolved
pkg/oplabels/labels.go Outdated Show resolved Hide resolved
pkg/oplabels/labels.go Outdated Show resolved Hide resolved
@jeffbanks jeffbanks requested a review from burmanm June 6, 2022 15:38
pkg/oplabels/labels.go Outdated Show resolved Hide resolved
Copy link
Contributor

@adejanovski adejanovski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few changes, and you'll also need to clean this label.

And I have a few tests in my PR that you can cherry pick.

pkg/oplabels/labels.go Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
@jeffbanks
Copy link
Contributor Author

@burmanm @emerkle826 @adejanovski

In the latest push: Added support for handling the label cassandradatacenter_controller as well as addition of label clean to be part of the api, which follows pattern of CleanupForKubernetes usage for other resources.

Update CHANGELOG.md
Co-authored-by: Alexander Dejanovski <[email protected]>
Support for CreatedByLabelValue; clean labels in api
Smoke test updates
Copy link
Contributor

@adejanovski adejanovski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, pending tests passing successfully.

apis/cassandra/v1beta1/cassandradatacenter_types.go Outdated Show resolved Hide resolved
pkg/oplabels/labels_test.go Outdated Show resolved Hide resolved
jeffbanks and others added 2 commits June 13, 2022 11:13
@jeffbanks
Copy link
Contributor Author

@adejanovski thanks for the quick review and keeping the comments accurate to the label values change!

@jeffbanks jeffbanks enabled auto-merge (squash) June 13, 2022 21:23
@jeffbanks
Copy link
Contributor Author

Label values validated in recent changes.

@jeffbanks jeffbanks closed this Jun 13, 2022
auto-merge was automatically disabled June 13, 2022 21:26

Pull request was closed

@jeffbanks jeffbanks reopened this Jun 13, 2022
@adejanovski
Copy link
Contributor

@jeffbanks, can we move forward and merge this PR?

@adejanovski adejanovski dismissed burmanm’s stale review June 14, 2022 13:48

Comments were resolved

@jeffbanks jeffbanks merged commit 34648fd into k8ssandra:master Jun 14, 2022
@jeffbanks jeffbanks deleted the jeffb/labels branch June 14, 2022 13:50
burmanm added a commit that referenced this pull request Jul 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

K8SSAND-1523 ⁃ Label values are not validated when created
3 participants