Get basic auth from secrets #628
Conversation
…g precedence over ConfigMaps.
This reverts commit 8cce767.
WalkthroughAdds optional Changes
Sequence DiagramsequenceDiagram
participant Client as Client (HTTP)
participant Controller as REST API Controller
participant Auth as Auth Helper
participant Secret as Kubernetes Secret
participant ConfigMap as ConfigMap (Fallback)
Client->>Controller: Incoming request
activate Controller
Controller->>Auth: addAuthToRequest(gateway)
activate Auth
rect rgb(220,240,220)
Note over Auth,Secret: Step 1 — Try Secret-based auth
Auth->>Secret: Fetch Secret referenced by authSecretRef
alt Secret found & `users.yaml` valid
Secret-->>Auth: users.yaml -> AuthSettings (Basic enabled)
else Secret missing/invalid
Secret-->>Auth: error / no users
end
end
alt No valid AuthSettings from Secret
rect rgb(255,245,220)
Note over Auth,ConfigMap: Step 2 — Fallback to ConfigMap
Auth->>ConfigMap: GetDeploymentConfigFromGateway(configRef)
ConfigMap-->>Auth: AuthSettings (if present)
end
end
Auth->>Controller: Authorization header / AuthSettings
deactivate Auth
Controller->>Client: Forwarded request with auth
deactivate Controller
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
kubernetes/gateway-operator/internal/auth/auth_helper_test.go (1)
80-129: LGTM! Test correctly validates Secret-based auth retrieval.The test properly sets up the scheme, creates a Secret with
users.yaml, and validates thatGetAuthConfigFromSecretreturns the expected authentication settings.Optional: Consider adding error-case tests
While the happy path is well-covered, consider adding tests for error scenarios to improve coverage:
- Missing Secret (AuthSecretRef points to non-existent Secret)
- Missing
users.yamlkey in Secret- Invalid YAML in
users.yamlThese would verify the error handling paths in
GetAuthConfigFromSecret.kubernetes/gateway-operator/internal/auth/auth_helper.go (1)
132-134: Clarify or remove the misleading stringData comment.The comment mentions "Fallback to stringData if not in Data (though client normally consolidates them)" but doesn't implement a fallback. This comment is misleading because when reading Secrets via controller-runtime or client-go, the
Datafield always contains the consolidated values—stringDatais only used for input during Secret creation/updates and is never returned by the API.🔎 Proposed fix to remove the misleading comment
// Look for users.yaml key usersYAML, ok := secret.Data["users.yaml"] if !ok { - // Fallback to stringData if not in Data (though client normally consolidates them) - // But in controller-runtime Struct Data contains byte slices return nil, fmt.Errorf("secret %s/%s does not contain 'users.yaml' key", gateway.Namespace, gateway.Spec.AuthSecretRef.Name) }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
go.workis excluded by!**/*.workgo.work.sumis excluded by!**/*.sum
📒 Files selected for processing (7)
.github/workflows/operator-integration-test.ymlkubernetes/gateway-operator/api/v1alpha1/gateway_types.gokubernetes/gateway-operator/api/v1alpha1/zz_generated.deepcopy.gokubernetes/gateway-operator/config/crd/bases/gateway.api-platform.wso2.com_gateways.yamlkubernetes/gateway-operator/internal/auth/auth_helper.gokubernetes/gateway-operator/internal/auth/auth_helper_test.gokubernetes/gateway-operator/internal/controller/restapi_controller.go
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-19T06:57:38.504Z
Learnt from: tharindu1st
Repo: wso2/api-platform PR: 514
File: gateway/gateway-controller/config/config.yaml:0-0
Timestamp: 2025-12-19T06:57:38.504Z
Learning: In gateway/gateway-controller/config/config.yaml, the default admin user with plaintext password "admin" is intentionally provided for testing purposes only and must be changed to secure hashed passwords for production deployments.
Applied to files:
.github/workflows/operator-integration-test.yml
🧬 Code graph analysis (3)
kubernetes/gateway-operator/internal/auth/auth_helper_test.go (3)
kubernetes/gateway-operator/api/v1alpha1/groupversion_info.go (1)
AddToScheme(35-35)kubernetes/gateway-operator/api/v1alpha1/gateway_types.go (2)
Gateway(266-272)GatewaySpec(101-134)kubernetes/gateway-operator/internal/auth/auth_helper.go (1)
GetAuthConfigFromSecret(114-153)
kubernetes/gateway-operator/internal/auth/auth_helper.go (1)
kubernetes/gateway-operator/api/v1alpha1/gateway_types.go (1)
Gateway(266-272)
kubernetes/gateway-operator/internal/controller/restapi_controller.go (1)
kubernetes/gateway-operator/internal/auth/auth_helper.go (3)
AuthSettings(54-57)GetAuthConfigFromSecret(114-153)GetDeploymentConfigFromGateway(80-109)
🔇 Additional comments (7)
kubernetes/gateway-operator/api/v1alpha1/zz_generated.deepcopy.go (1)
250-254: LGTM! Auto-generated deepcopy follows the correct pattern.The nil-safe deepcopy for
AuthSecretRefcorrectly mirrors the existingConfigRefpattern and is properly generated by controller-gen..github/workflows/operator-integration-test.yml (2)
836-850: LGTM! Secret-based authentication properly configured for integration tests.The Secret structure correctly provides
users.yamlwith admin credentials, and the Gateway CR properly references it viaauthSecretRef.Based on learnings, the plaintext admin credentials are intentionally used for testing purposes.
Also applies to: 865-866
1266-1280: LGTM! Scoped-test namespace correctly includes Secret-based auth.The Secret and Gateway configuration for the scoped-test namespace mirrors the default namespace setup, ensuring consistent test coverage.
Also applies to: 1294-1295
kubernetes/gateway-operator/api/v1alpha1/gateway_types.go (1)
128-133: LGTM! AuthSecretRef field properly defined.The new optional field is correctly typed, documented, and follows the existing
ConfigRefpattern. The documentation clearly states the precedence behavior.kubernetes/gateway-operator/internal/controller/restapi_controller.go (1)
877-897: LGTM! Secret-first fallback logic correctly implements precedence.The authentication retrieval properly implements the two-tier fallback:
- Attempts
GetAuthConfigFromSecret(Secret-based viaAuthSecretRef)- Falls back to
GetDeploymentConfigFromGateway(ConfigMap-based viaConfigRef) if Secret is absent or fails- Uses default credentials if both fail
The error handling appropriately logs warnings while continuing to fallback options, ensuring the gateway remains operational even with configuration issues.
kubernetes/gateway-operator/internal/auth/auth_helper.go (1)
111-153: LGTM!The implementation correctly retrieves and parses authentication configuration from the referenced Secret. The function appropriately:
- Returns nil when no AuthSecretRef is specified
- Fetches the Secret with proper error handling
- Validates the presence of the required
users.yamlkey- Parses the YAML into the expected user structure
- Constructs AuthSettings with Basic authentication enabled
kubernetes/gateway-operator/config/crd/bases/gateway.api-platform.wso2.com_gateways.yaml (1)
106-122: LGTM!The
authSecretReffield definition follows Kubernetes conventions for referencing Secrets. The schema correctly:
- Makes the field optional to maintain backward compatibility
- Documents the expected
users.yamlkey and precedence over ConfigRef- Uses the standard LocalObjectReference structure
- Applies the
x-kubernetes-map-type: atomicmarker appropriately
1 participant
Purpose
Summary by CodeRabbit
New Features
Tests