Skip to content

MLE-33163 : provision operator user and implement test - #213

Open
rwinieski wants to merge 18 commits into
feature/1.4.0from
MLE-33163/Provision-Operator-User
Open

rwinieski wants to merge 18 commits into
feature/1.4.0from
MLE-33163/Provision-Operator-User

Conversation

@rwinieski

Copy link
Copy Markdown
Collaborator

This pull request introduces support for a dedicated MarkLogic Kubernetes Operator user and credential management, enhancing security and least-privilege practices. It adds the ability to use a user-managed operator Secret, documents the new behavior, and updates the controller logic and CRDs to track and react to operator credential changes. Comprehensive tests are included to verify correct group reconciliation on Secret updates.

Operator credential management and role separation:

  • Added support for a dedicated MarkLogic user (marklogic-kubernetes-operator) and role (marklogic-operator) with least-privilege access for Management API operations; the Operator manages this user and its credential, stored in a cluster-owned Secret, and falls back to the admin Secret for recovery if needed. [1] [2]
  • Introduced the operatorSecretName field in the CRD and AdminAuth struct, allowing users to supply their own Secret for the operator credential, and updated all related CRDs and sample manifests with documentation and schema changes. [1] [2] [3] [4] [5] [6]

Status tracking and reconciliation:

  • Added credentialSecretName to MarklogicGroupStatus and CRD to record the active operator credential Secret after bootstrap handoff. [1] [2]

Controller logic enhancements:

  • Updated the controller to watch for Secret updates and reconcile affected MarklogicGroup resources, including a new mapping function to associate Secrets with groups based on ownership or reference. [1] [2] [3]

Testing:

  • Added unit tests for Secret-to-group mapping and event filtering to ensure correct reconciliation behavior when operator credentials are rotated or updated. [1] [2] [3]

Documentation:

  • Updated the README.md and design documentation to explain the new operator user, credential handoff, rotation procedures, and least-privilege rationale. [1] [2]

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

Credential sequencing, static joins, Helm CRDs, readiness checks, and rollout behavior contain blocking defects.

Review effort: Balanced
Findings: 3 High severity · 4 Medium severity

Open (7)
What changed in this PR

Introduces dedicated least-privilege operator credentials, credential handoff/rotation, and Secret-driven reconciliation.

Changes:

  • Adds operator role/user provisioning and credential Secrets.
  • Adds status tracking, Secret watches, and serialized pod rotation.
  • Adds documentation and unit/E2E coverage.
File Description
test/​utils/​operator_credentials.go Adds handoff verification helpers.
test/​e2e/​main_test.go Registers the new test namespace.
test/​e2e/​11_operator_credentials_test.go Tests credential handoff end-to-end.
test/​e2e-helm/​main_test.go Adds the Helm test namespace.
test/​e2e-helm/​10_operator_credentials_test.go Tests handoff under Helm deployment.
README.md Documents operator credentials and rotation.
pkg/​mlmanage/​client.go Adds operator role/user APIs.
pkg/​mlmanage/​client_test.go Tests role and user reconciliation.
pkg/​k8sutil/​statefulset.go Mounts credentials and rotates pods.
pkg/​k8sutil/​secret.go Reconciles operator credential Secrets.
pkg/​k8sutil/​secret_test.go Tests Secret and identity reconciliation.
pkg/​k8sutil/​scripts/​prestop-hook.sh Uses operator credentials for shutdown.
pkg/​k8sutil/​scripts/​copy-certs.sh Selects credentials after handoff.
pkg/​k8sutil/​scripts/​cluster-config.sh Updates join credentials and validation.
pkg/​k8sutil/​operator_user.go Implements identity handoff and recovery.
pkg/​k8sutil/​marklogicServer_test.go Tests serialized credential rotation.
pkg/​k8sutil/​handler.go Adds operator-user reconciliation.
pkg/​k8sutil/​dynamic_reconcile.go Uses operator credentials for dynamic hosts.
pkg/​k8sutil/​dynamic_reconcile_test.go Updates dynamic reconciliation tests.
internal/​controller/​marklogicgroup_controller.go Watches credential Secrets.
internal/​controller/​marklogicgroup_controller_test.go Tests Secret mapping and predicates.
docs/​spec/​Dynamic Host.md Updates credential separation design.
config/​samples/​quick-start.yaml Documents optional operator Secret.
config/​samples/​minimal-production.yaml Adds operator Secret guidance.
config/​samples/​complete.yaml Adds operator Secret guidance.
config/​crd/​bases/​marklogic.progress.com_marklogicgroups.yaml Extends group schema and status.
config/​crd/​bases/​marklogic.progress.com_marklogicclusters.yaml Extends cluster authentication schema.
api/​v1/​zz_generated.deepcopy.go Copies the new authentication field.
api/​v1/​marklogicgroup_types.go Adds active credential status.
api/​v1/​common_types.go Adds operatorSecretName.
Files not reviewed (1)
  • api/v1/zz_generated.deepcopy.go: Generated file

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread config/crd/bases/marklogic.progress.com_marklogicclusters.yaml
Comment thread pkg/k8sutil/scripts/cluster-config.sh Outdated
Comment thread pkg/k8sutil/statefulset.go Outdated
Comment thread pkg/k8sutil/handler.go Outdated
Comment thread pkg/k8sutil/scripts/prestop-hook.sh Outdated
Comment thread pkg/k8sutil/statefulset.go
Comment thread pkg/k8sutil/statefulset.go Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

The role payload, missing-Secret fallback, credential quoting, and stale readiness assertion can prevent successful handoff or testing.

Review effort: Balanced
Findings: 2 High severity · 2 Medium severity

Open (4)
Resolved since last review (7)
Files not reviewed (1)
  • api/v1/zz_generated.deepcopy.go: Generated file
Previously missed (1)

In code that hasn't changed since last review

Medium severity Normalize operator Secret names in watch mappings

internal/​controller/​marklogicgroup_controller.go:236

Secret resolution trims operatorSecretName, but this watch mapping compares the raw value. A value such as " custom-operator-auth " is accepted and read successfully by reconciliation, yet rotations of custom-operator-auth never enqueue the group. Apply the same strings.TrimSpace normalization here (and to the admin Secret comparison) so watch behavior matches lookup behavior.

Comment thread internal/controller/marklogicgroup_controller_test.go Outdated
Comment thread pkg/mlmanage/client.go Outdated
Comment thread pkg/k8sutil/handler.go
Comment thread pkg/k8sutil/scripts/cluster-config.sh Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

Credential recovery, pod rotation safety, shell quoting, and an outdated test assertion need correction.

Review effort: Balanced
Findings: 2 High severity · 2 Medium severity

Open (4)
Resolved since last review (1)
Files not reviewed (1)
  • api/v1/zz_generated.deepcopy.go: Generated file
Previously missed (1)

In code that hasn't changed since last review

Medium severity Wait for replacement pod before deleting another stale pod

pkg/​k8sutil/​statefulset.go:229

After deleting one stale pod, the next reconcile may run before its replacement appears in the list. In that window every listed peer can be Ready, so another stale pod is deleted and the promised one-at-a-time rotation can reduce availability. Wait until the observed pod count is back at the desired replica count; then the existing peer-readiness check also ensures the replacement is Ready.

Comment thread internal/controller/marklogicgroup_controller.go

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

Secret update filtering and the all-host join gate can prevent credential repair and pod recovery.

Review effort: Balanced
Findings: 2 High severity

Open (2)
Resolved since last review (4)
Files not reviewed (1)
  • api/v1/zz_generated.deepcopy.go: Generated file
Previously missed (2)

In code that hasn't changed since last review

Medium severity Trim operator Secret name before event mapping

internal/​controller/​marklogicgroup_controller.go:236

Credential lookup normalizes operatorSecretName with TrimSpace, but this event mapper compares the untrimmed value. A value such as " custom-operator-auth " is therefore read successfully as custom-operator-auth, yet later Secret rotations do not enqueue the group, so the MarkLogic password and pod revision remain stale. Normalize the reference consistently here.

Medium severity Require only the joined host to be online

pkg/​k8sutil/​scripts/​cluster-config.sh:175

This gate requires every host in the cluster to be online, not just the host that has just joined. During a partial outage or planned maintenance, an unrelated offline host therefore makes a new/restarting pod fail its startup script after a successful join, reducing recovery capacity. Validate the joined host's own online status instead of total-hosts-offline == 0.

Comment thread internal/controller/marklogiccluster_controller.go

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🔵 Needs a closer look

Admin fallback cannot rotate existing OnDelete pods after the active operator Secret disappears.

Review effort: Balanced
Findings: 1 High severity

Open (1)
Resolved since last review (1)
Files not reviewed (1)
  • api/v1/zz_generated.deepcopy.go: Generated file
Previously missed (1)

In code that hasn't changed since last review

Medium severity Preserve credential state to rotate stale OnDelete pods during admin fallback

pkg/​k8sutil/​statefulset.go:210

Clearing the active credential status here breaks admin fallback for OnDelete StatefulSets. This reconcile updates only the pod template to use the admin Secret; the existing pods are not restarted automatically. Subsequent reconciles cannot rotate them because rotateCredentialPodsIfNeeded requires a non-empty CredentialSecretName, and the handler returns early while the operator Secret remains missing. Those pods therefore keep MARKLOGIC_OPERATOR_CREDENTIALS_ACTIVE=true with a vanished optional Secret, so pre-stop shutdown cannot authenticate. Preserve enough fallback/rotation state and allow the handler to serially replace stale OnDelete pods before considering the handoff cleared.

This branch has not been deployed

No deployments
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.

2 participants