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

MC-1296 Deploy Reaper capable of interaction with encrypted mgmt-api #1421

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

rzvoncek
Copy link
Contributor

@rzvoncek rzvoncek commented Sep 30, 2024

What this PR does:
This PR changes the reconciliation process of k8ssandra cluster and reaper objects so that it becomes possible for one a single reaper insance to interact with several k8ssandra clusters with their mgmt-api communication encrypted (each using a different certificate).

Which issue(s) this PR fixes:
Fixes

Checklist

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

@rzvoncek rzvoncek requested a review from a team as a code owner September 30, 2024 14:29
Copy link

No linked issues found. Please add the corresponding issues in the pull request description.
Use GitHub automation to close the issue when a PR is merged

@rzvoncek rzvoncek force-pushed the MC-1296-mount-mgtmapi-certs-to-reaper branch from 96a2119 to f3d85c7 Compare October 22, 2024 08:58
Copy link
Member

@Miles-Garnsey Miles-Garnsey left a comment

Choose a reason for hiding this comment

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

@rzvoncek when I run CreateControlPlaneReaperAndDataCenter from vs code, I'm seeing the test fail -

--- FAIL: TestOperator (926.60s)
    --- FAIL: TestOperator/CreateControlPlaneReaperAndDataCenter (926.60s)
        /Users/milesgarnsey/projects/k8ssandra-operator/test/e2e/e2e_framework.go:69: Using config file: /Users/milesgarnsey/projects/k8ssandra-operator/build/kind-kubeconfig
        /Users/milesgarnsey/projects/k8ssandra-operator/test/e2e/e2e_framework.go:77: Using control plane: kind-k8ssandra-0
        /Users/milesgarnsey/projects/k8ssandra-operator/test/e2e/e2e_framework.go:94: Using data planes: [kind-k8ssandra-0 kind-k8ssandra-1]
        /Users/milesgarnsey/projects/k8ssandra-operator/test/e2e/reaper_test.go:350: check that Reaper reaper1 in cluster kind-k8ssandra-0 is ready
        /Users/milesgarnsey/projects/k8ssandra-operator/test/e2e/suite_test.go:1805: check that datacenter enc-mgmt-dc1 in cluster kind-k8ssandra-0 is ready
        /Users/milesgarnsey/projects/k8ssandra-operator/test/e2e/suite_test.go:1805: check that datacenter enc-mgmt-2-dc1 in cluster kind-k8ssandra-0 is ready
        /Users/milesgarnsey/projects/k8ssandra-operator/test/e2e/reaper_test.go:321: Verify Reaper received k8ssandra-cluster secrets
        /Users/milesgarnsey/projects/k8ssandra-operator/test/e2e/suite_test.go:1951: 
            	Error Trace:	/Users/milesgarnsey/projects/k8ssandra-operator/test/e2e/suite_test.go:1951
            	            				/Users/milesgarnsey/projects/k8ssandra-operator/test/e2e/reaper_test.go:322
            	            				/Users/milesgarnsey/projects/k8ssandra-operator/test/e2e/suite_test.go:481
            	Error:      	Received unexpected error:
            	            	secrets "reaper1-truststores" not found
            	Test:       	TestOperator/CreateControlPlaneReaperAndDataCenter
            	Messages:   	failed to get reaper's truststore secret
        /Users/milesgarnsey/projects/k8ssandra-operator/test/e2e/suite_test.go:776: failed to delete namespace: context deadline exceeded
FAIL
FAIL	github.com/k8ssandra/k8ssandra-operator/test/e2e	927.122s

Is there something extra I need to do to make this work?

@rzvoncek
Copy link
Contributor Author

This led me to notice the test I added did not actually ran with the CI ... I've added it and it now ran here. It seems to have passed. The PR adds a bunch of stuff, but it seems to be all present in the changed files.

So I'm not sure why this would fail ... when running from an IDE. I always run the e2e tests from shell, with:

e2e_test="TestOperator/CreateControlPlaneReaperAndDataCenter"
make single-up
make E2E_TEST="$e2e_test" e2e-test

Could you please try that way?

Copy link
Member

@Miles-Garnsey Miles-Garnsey 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 things:

  1. I think you need to create new secrets for the sake of security, not re-use existing client secrets.
  2. Unit/integration tests are currently failing.
  3. I'm still not able to run CreateControlPlaneReaperAndDataCenter successfully. I also checked that other tests work. AddDcToClusterSameDataplane definitely works when run this way so it isn't just because we're running from the IDE.

If I'm understanding this correctly there are two sets of changes. One set in the Reaper controller creates mounts and an empty secret. Another set in the k8ssandra cluster controller adds the existing client encryption materials to Reaper's secret.

I don't fully understand the API on the Reaper side yet, but this looks OK generally to me, just have a look at some of my comments throughout and the three main things I've highlighted above.

controllers/k8ssandra/reaper.go Show resolved Hide resolved
controllers/k8ssandra/reaper.go Show resolved Hide resolved
controllers/k8ssandra/reaper.go Show resolved Hide resolved
pkg/reaper/secrets.go Show resolved Hide resolved
controllers/k8ssandra/reaper.go Show resolved Hide resolved
controllers/reaper/reaper_controller.go Show resolved Hide resolved
controllers/reaper/reaper_controller_test.go Outdated Show resolved Hide resolved
controllers/reaper/reaper_controller_test.go Outdated Show resolved Hide resolved
test/e2e/suite_test.go Show resolved Hide resolved
@burmanm
Copy link
Contributor

burmanm commented Oct 25, 2024

I think you need to create new secrets for the sake of security, not re-use existing client secrets.

This is not the correct way, as mgmt-api supports a single TLS cert (combination of tls.crt/ca.crt/tls.key files, not Java keystores). Thus, if Reaper doesn't use the cass-operator one, then Reaper will not be able to connect to mgmt-api.

@rzvoncek rzvoncek force-pushed the MC-1296-mount-mgtmapi-certs-to-reaper branch from 94b6bdd to c7c0baa Compare November 22, 2024 14:46
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.

4 participants