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

Add the ability to configure Reaper with truststore/keystore #1182

Merged
merged 2 commits into from
Jan 30, 2024

Conversation

burmanm
Copy link
Contributor

@burmanm burmanm commented Jan 22, 2024

What this PR does:
Adds keystore and truststore properties and their configuration and mounting to the Reaper Spec

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

Checklist

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

@burmanm burmanm requested a review from a team as a code owner January 22, 2024 13:50
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions

5.8% Duplication on New Code (required ≤ 3%)

See analysis details on SonarCloud

Copy link

codecov bot commented Jan 22, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (ff37de8) 57.19% compared to head (bf79c2e) 57.36%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1182      +/-   ##
==========================================
+ Coverage   57.19%   57.36%   +0.16%     
==========================================
  Files         101      101              
  Lines       10529    10554      +25     
==========================================
+ Hits         6022     6054      +32     
+ Misses       3980     3975       -5     
+ Partials      527      525       -2     
Files Coverage Δ
pkg/reaper/deployment.go 94.20% <100.00%> (+0.47%) ⬆️

... and 4 files with indirect coverage changes

Copy link
Contributor

@olim7t olim7t left a comment

Choose a reason for hiding this comment

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

LGTM, but we should test to validate the boolean change.

@@ -378,7 +378,9 @@ type HttpManagement struct {
// When enabled, HTTP will be used instead of JMX for management connectivity between Cassandra
// and Reaper. In future, this will be true by default
// +kubebuilder:default=false
Enabled *bool `json:"enabled"`
Enabled bool `json:"enabled,omitempty"`
Copy link
Contributor

Choose a reason for hiding this comment

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

[thought] I would be careful changing that, since the comment says it might default to true in the future.

The problem with a raw boolean is that in some case it becomes impossible to differentiate between "unset" and "explicitly set to false".

@burmanm burmanm merged commit 14df237 into k8ssandra:main Jan 30, 2024
59 of 61 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add ability to configure keystore and truststore for Reaper http-api
2 participants