feat(cluster): add secret manager settings#2718
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## staging #2718 +/- ##
===========================================
+ Coverage 45.86% 46.06% +0.19%
===========================================
Files 1164 1191 +27
Lines 24497 24812 +315
Branches 7196 7304 +108
===========================================
+ Hits 11236 11430 +194
- Misses 11299 11400 +101
- Partials 1962 1982 +20
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Introduces cluster-level secret manager add-on settings behind a secret-manager PostHog feature flag, and reorganises cluster Add-ons (KEDA + secret manager) into a new /addons route. Also adds a new shared @qovery/shared/util-clusters lib, a secret manager modal with provider/constraint-aware UX, and bumps qovery-typescript-axios to 1.1.893.
Changes:
- New
settings/addonsroute exposing the KEDA toggle (moved out of General settings) and a feature-flagged secret manager section with provider-aware add modal and list. - New
secret-manager-integration-modal+secret-manager-integration-constraintsenforcing AWS/GCP automatic-vs-manual rules and OIDC requirements, plus new cluster data-access queries/hooks for provider secrets and associated external secrets. - New
@qovery/shared/util-clusterslib with cluster/provider/secret-manager helpers, and anInputSelectenhancement to render disabled-option tooltips.
Reviewed changes
Copilot reviewed 47 out of 48 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| package.json, yarn.lock | Bump qovery-typescript-axios to 1.1.893 for SecretManagerAccessApi. |
| tsconfig.base.json | Register @qovery/shared/util-clusters path. |
| libs/shared/util-clusters/** | New shared lib: cluster/provider predicates, secret-manager helpers + tests, lint/build config. |
| libs/shared/interfaces/src/lib/common/value.interface.ts | Add optional disabledTooltip to Value. |
| libs/shared/ui/.../input-select.tsx | Wrap option with tooltip + aria-disabled when option is disabled. |
| libs/domains/cloud-providers/.../use-cloud-providers.ts | Add optional suspense param. |
| libs/domains/clusters/data-access/.../domains-clusters-data-access.ts | Add listSecretManagerSecretsFromProvider and listSecretManagerAssociatedServices queries. |
| libs/domains/clusters/feature/src/lib/hooks/use-secret-manager-*/** | New hooks for provider secrets + associated services. |
| libs/domains/clusters/feature/src/lib/cluster-general-settings/** | Remove KEDA toggle (moved to Add-ons) and its test. |
| libs/domains/clusters/feature/src/lib/cluster-addons/** | New AddonToggleCard, SecretManagerList, secret-manager options + tests + barrel exports. |
| libs/domains/clusters/feature/src/lib/secret-manager-modals/** | New integration modal + constraints + types + tests. |
| libs/domains/clusters/feature/src/index.ts | Export new add-ons, modal, and hooks. |
| apps/console/src/routes/.../settings/addons.tsx | New Add-ons route handling KEDA + secret managers, feature-flagged. |
| apps/console/src/routes/.../settings/route.tsx | Add Add-ons sidebar entry for AWS/GCP clusters. |
| apps/console/src/routeTree.gen.ts | Generated route tree updates for /addons. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Thanks @rmnbrd, I added several comments!
- The modal’s top rounded corners aren’t visible
- Could we save directly via the API when clicking "Add"? I think it’s a bit confusing for end users, we don’t have this extra step in the Cloud Credentials flow, for example
https://www.loom.com/share/14ba8f3d2c61404d95000e7c81d8730c
| @@ -0,0 +1,4 @@ | |||
| import { type SecretManagerAccess } from 'qovery-typescript-axios' | |||
|
|
|||
| export const isAwsSecretManager = (secretManager: SecretManagerAccess | undefined) => | |||
There was a problem hiding this comment.
Small question, should isAwsSecretManager also return true for AWS_PARAMETER_STORE?
AWS_PARAMETER_STORE as an AWS option, but this helper only recognizes AWS_SECRET_MANAGER. So if a Parameter Store integration already exists with automatic or STS authentication, it looks like it would not be counted by the AWS exclusivity rules. Is that intentional?
There was a problem hiding this comment.
Yes, that's intentional.
| @@ -0,0 +1,12 @@ | |||
| export { isGcpCluster } from './is-gcp-cluster' | |||
There was a problem hiding this comment.
Should these helpers stay closer to the cluster domain instead of being introduced under @qovery/shared/util-clusters?
This seems to create a new shared utility library for cluster-specific logic, with helpers like isGcpCluster, isAwsCluster, and several Secret Manager predicates. Some of them are very small, so we end up with quite a few files containing only a few lines, and the cluster rules become spread across many entry points
For services, similar checks live closer to the domain/data-access layer (isHelm, isApplication, etc), so I wonder if we should keep the same pattern here and place these cluster checks under domains/clusters instead. If shared/util-clusters is the intended direction, maybe we should clarify when helpers belong there versus in the domain layer
| return ( | ||
| <div key={manager.id} className={`flex items-center justify-between gap-3 p-3 `}> | ||
| <div className="flex min-w-0 flex-1 items-center gap-3"> | ||
| <Icon name={getSecretManagerProvider(manager)} width={24} height={24} /> |
There was a problem hiding this comment.
Do you use this kind of function often in your follow‑up PRs?
getReadableSecretManagerAuth,
getReadableSecretManagerProvider,
getSecretManagerProvider
There was a problem hiding this comment.
What do you mean? What's your question?
Do you wanna know if the follow-up PRs add more similar helper functions or if these ones are re-used later one?
There was a problem hiding this comment.
They will be re-used, yes:
| @@ -0,0 +1,6 @@ | |||
| export type SecretManagerOption = { | |||
There was a problem hiding this comment.
Is it necessary to have a separate file for this? It feels like you only use it once
There was a problem hiding this comment.
It will be re-used in follow-up PRs
|
I've taken into account your suggestions and answered your questions. |
Summary
Adds the secret manager foundation at cluster settings level: API/client updates, cluster data-access hooks, shared secret manager add-on UI, modal/constraint handling, and the cluster settings add-ons route integration.
Screenshots / Recordings
Testing
yarn testoryarn test -u(if you need to regenerate snapshots)yarn formatyarn lintPR Checklist
.cursor/rules)feat(service): add new Terraform service) - required for semantic-release