Add workload.yaml descriptors for OpenChoreo source builds#431
Add workload.yaml descriptors for OpenChoreo source builds#431isala404 wants to merge 2 commits intoOpenDIF:mainfrom
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the deployability and management of several core services and portals within the OpenChoreo platform by introducing dedicated workload.yaml descriptors. These new configurations standardize how these components interact, manage secrets, and expose endpoints, streamlining their integration into the OpenChoreo ecosystem. Additionally, a minor adjustment was made to the portal backend's database migration settings. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces workload.yaml descriptors for eight services. A critical vulnerability has been identified: multiple components have database SSL disabled in production-like environments, and the portal backend is configured with a 'fail-open' authorization mode. Additionally, the review found several critical and high-severity issues including hardcoded placeholder secrets in portal-backend/workload.yaml, the use of internal-only Kubernetes service URLs in frontend portal configurations (which will break user authentication), configurations that will likely lead to non-functional portals, and brittle service-to-service communication due to hardcoded URLs. Addressing these points will improve the security, correctness, and maintainability of the services.
| - name: ASGARDEO_CLIENT_SECRET | ||
| value: placeholder | ||
| - name: CHOREO_PDP_CONNECTION_CHOREOAPIKEY | ||
| value: placeholder |
There was a problem hiding this comment.
The environment variables ASGARDEO_CLIENT_SECRET and CHOREO_PDP_CONNECTION_CHOREOAPIKEY are hardcoded with the value "placeholder". This is a critical security vulnerability. Secrets must not be hardcoded in configuration files. They should be injected securely at runtime by referencing a Kubernetes secret, similar to how database credentials are handled in this file. The secret names and keys in the suggestion below are examples and should be adjusted to match your actual secret definitions.
- name: ASGARDEO_CLIENT_SECRET
valueFrom:
secretKeyRef:
name: ndx-asgardeo-secrets
key: client-secret
- name: CHOREO_PDP_CONNECTION_CHOREOAPIKEY
valueFrom:
secretKeyRef:
name: ndx-pdp-secrets
key: api-key| - name: VITE_IDP_BASE_URL | ||
| value: http://thunder-service.thunder.svc.cluster.local:8090 |
There was a problem hiding this comment.
The VITE_IDP_BASE_URL is hardcoded to a Kubernetes internal service URL (http://thunder-service.thunder.svc.cluster.local:8090). This URL is not accessible from a user's browser, which will prevent authentication from working. This variable must be set to the public-facing URL of the identity provider.
value: "https://your-public-idp.com" # Replace with the actual public URL| - name: VITE_BASE_URL | ||
| value: http://thunder-service.thunder.svc.cluster.local:8090 |
There was a problem hiding this comment.
The VITE_BASE_URL for the IDP is hardcoded to a Kubernetes internal service URL (http://thunder-service.thunder.svc.cluster.local:8090). This URL is not accessible from a user's browser, which will prevent authentication from working. This variable must be set to the public-facing URL of the identity provider.
value: "https://your-public-idp.com" # Replace with the actual public URL| - name: VITE_BASE_URL | ||
| value: http://thunder-service.thunder.svc.cluster.local:8090 |
There was a problem hiding this comment.
The VITE_BASE_URL for the IDP is hardcoded to a Kubernetes internal service URL (http://thunder-service.thunder.svc.cluster.local:8090). This URL is not accessible from a user's browser, which will prevent authentication from working. This variable must be set to the public-facing URL of the identity provider.
value: "https://your-public-idp.com" # Replace with the actual public URL| - name: AUTHORIZATION_MODE | ||
| value: fail_open_admin |
There was a problem hiding this comment.
The authorization mode is set to fail_open_admin. "Fail-open" is a security anti-pattern where access is granted if the authorization check fails or cannot be completed. In a security-sensitive application, the system should always "fail-closed," denying access by default if an error occurs during the authorization process to prevent unauthorized access.
| - name: VITE_CONSENT_ENGINE_URL | ||
| value: "" | ||
| - name: VITE_API_URL | ||
| value: "" | ||
| - name: VITE_CLIENT_ID | ||
| value: NDX_CONSENT_PORTAL | ||
| - name: VITE_BASE_URL | ||
| value: http://thunder-service.thunder.svc.cluster.local:8090 | ||
| - name: VITE_SCOPE | ||
| value: "openid profile email" | ||
| - name: VITE_SIGN_IN_REDIRECT_URL | ||
| value: "" | ||
| - name: VITE_SIGN_OUT_REDIRECT_URL | ||
| value: "" |
There was a problem hiding this comment.
Several configuration variables (VITE_CONSENT_ENGINE_URL, VITE_API_URL, VITE_SIGN_IN_REDIRECT_URL, VITE_SIGN_OUT_REDIRECT_URL) are set to empty strings. This will likely render the portal non-functional. These values must be configured with appropriate URLs for the portal to work correctly. For example, VITE_API_URL should be populated using the NDX_PORTAL_BACKEND_URL from the connection definition, and the redirect URLs must be set to the portal's public address.
| - name: VITE_API_URL | ||
| value: "" | ||
| - name: VITE_LOGS_URL | ||
| value: "" | ||
| - name: VITE_CLIENT_ID | ||
| value: NDX_MEMBER_PORTAL | ||
| - name: VITE_BASE_URL | ||
| value: http://thunder-service.thunder.svc.cluster.local:8090 | ||
| - name: VITE_SCOPE | ||
| value: "openid profile email groups" | ||
| - name: VITE_SIGN_IN_REDIRECT_URL | ||
| value: "" | ||
| - name: VITE_SIGN_OUT_REDIRECT_URL | ||
| value: "" |
There was a problem hiding this comment.
Several configuration variables (VITE_API_URL, VITE_LOGS_URL, VITE_SIGN_IN_REDIRECT_URL, VITE_SIGN_OUT_REDIRECT_URL) are set to empty strings. This will likely render the portal non-functional. These values must be configured with appropriate URLs for the portal to work correctly. For example, VITE_API_URL should be populated using the NDX_PORTAL_BACKEND_URL from the connection definition, and the redirect URLs must be set to the portal's public address.
| - name: DB_SSLMODE | ||
| value: disable |
There was a problem hiding this comment.
The database SSL mode is explicitly disabled (DB_SSLMODE: disable) while the environment is set to production. This is a significant security risk as it allows for potential eavesdropping and man-in-the-middle attacks on sensitive data. It is strongly recommended to enable SSL for all database connections in production.
value: require| - name: DB_SSLMODE | ||
| value: disable |
There was a problem hiding this comment.
The database SSL mode is explicitly disabled (DB_SSLMODE: disable) while the environment is set to production. This is a significant security risk as it allows for potential eavesdropping and man-in-the-middle attacks on sensitive data. Please enable SSL for the database connection.
value: require| - name: DB_SSLMODE | ||
| value: disable |
There was a problem hiding this comment.
The database SSL mode is explicitly disabled (DB_SSLMODE: disable). Disabling SSL for database connections is a security risk as it allows for potential eavesdropping and man-in-the-middle attacks on sensitive data. In any environment handling sensitive data, SSL should be enabled. Please enable SSL for this connection.
value: require1 participant
Summary
workload.yamlOpenChoreo descriptors for all 8 components: audit-service, consent-engine, orchestration-engine, policy-decision-point, portal-backend, admin-portal, consent-portal, and member-portalopenchoreo.dev/v1alpha1APIconfig.jsonfile for runtime configurationTest plan