Skip to content

Commit 2b57f1a

Browse files
MikelAlejoBRMikelAlejoBR
committed
docs: design of the "enabled integrations" feature
These are the design documents used for the "enabled integrations" feature, which aims at showing or hiding the available integrations in Sandbox depending on the configuration specified in the ToolchainConfig resource. SANDBOX-1769
1 parent 4ff1ff9 commit 2b57f1a

File tree

2 files changed

+297
-0
lines changed

2 files changed

+297
-0
lines changed

‎docs/proposals/enabled-integrations-design.md‎

Lines changed: 146 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,146 @@
1+
# Integration Visibility Configuration
2+
3+
**Status:** Final
4+
5+
## Overview
6+
7+
Developer Sandbox integrates with several downstream services (e.g. OpenShift Lightspeed, DevSpaces, Workato, etc.). When one of these downstream integrations experiences issues, there is currently no way to hide it from customers without a code change and redeployment.
8+
9+
This design adds a `disabledIntegrations` field to `ToolchainConfig` so that operators can dynamically hide broken integrations from users. The registration service exposes this list via the existing `/api/v1/uiconfig` endpoint, allowing the UI to conditionally hide integration entry points at runtime.
10+
11+
The field uses a **denylist** approach: the normal state is an empty field (all integrations enabled), and admins only add the 1-2 broken integrations when something goes wrong. This avoids the operational burden of maintaining a full allowlist of every integration.
12+
13+
## Design Principles
14+
15+
- **Minimal API surface change**: add a single new field to the existing `ToolchainConfig` CRD rather than introducing a new CRD.
16+
- **Safe defaults**: when the field is not set, all integrations are enabled (backward compatible with existing deployments).
17+
- **Operational simplicity**: the common case (everything healthy) requires zero configuration; only broken integrations are listed.
18+
- **Consistency with existing patterns**: follows the same conventions used by `UICanaryDeploymentWeight` and `WorkatoWebHookURL` for CRD placement and REST exposure.
19+
- **Decoupled from feature toggles**: feature toggles control per-Space probabilistic template rendering; this mechanism controls UI-level visibility of integrations for all users.
20+
21+
## Architecture / How It Works
22+
23+
```
24+
┌──────────────┐ ┌──────────────────┐ ┌─────────────────────┐
25+
│ Cluster │ │ Registration │ │ Sandbox UI │
26+
│ Admin │──────▶│ Service │──────▶│ (Browser) │
27+
│ │ │ │ │ │
28+
│ applies │ │ GET /api/v1/ │ │ fetches uiconfig, │
29+
│ ToolchainCfg │ │ uiconfig │ │ reads disabled- │
30+
└──────────────┘ │ (JWT-secured) │ │ Integrations, │
31+
│ └──────────────────┘ │ hides those │
32+
│ ▲ └─────────────────────┘
33+
▼ │
34+
┌──────────────────────────┐ │
35+
│ToolchainConfig CR │ │
36+
│ spec.host │ │
37+
│ .registrationService │ │
38+
│ .disabledIntegrations:│ │
39+
│ - "lightspeed" │────┘
40+
└──────────────────────────┘
41+
(loaded via commonconfig.LoadLatest)
42+
```
43+
44+
1. **Cluster admin** updates the `ToolchainConfig` CR, adding broken integrations to `disabledIntegrations`.
45+
2. **Registration service** picks up the change via its cached client (same mechanism used for all other config — `commonconfig.LoadLatest`).
46+
3. **`GET /api/v1/uiconfig`** (JWT-secured) serves the disabled list alongside existing fields (`uiCanaryDeploymentWeight`, `workatoWebHookURL`).
47+
4. **UI** fetches the endpoint and hides any integration present in the `disabledIntegrations` array.
48+
49+
## Core Concepts
50+
51+
### Integration identifier
52+
53+
Each integration is represented by a **string name** (e.g. `"openshift-lightspeed"`, `"devspaces"`, `"workato"`). These are free-form identifiers agreed upon between the backend configuration and the UI.
54+
55+
### Default behavior
56+
57+
When `disabledIntegrations` is nil or empty, **all integrations are enabled**. This preserves backward compatibility — existing deployments without the field continue to work with everything visible. Admins only populate the field when they need to hide something.
58+
59+
### Field location
60+
61+
The `disabledIntegrations` field lives under `spec.host.registrationService` in the `ToolchainConfig` CRD, co-located with other UI-facing configuration (`uiCanaryDeploymentWeight`, `workatoWebHookURL`).
62+
63+
### CRD field definition
64+
65+
```go
66+
// In RegistrationServiceConfig (api/v1alpha1/toolchainconfig_types.go):
67+
68+
// DisabledIntegrations specifies the list of integrations that should be
69+
// hidden/disabled in the UI. When nil or empty, all integrations are
70+
// considered enabled. Only listed integrations are hidden.
71+
// +optional
72+
// +listType=set
73+
DisabledIntegrations []string `json:"disabledIntegrations,omitempty"`
74+
```
75+
76+
### REST response
77+
78+
The existing `UIConfigResponse` in the registration service is extended with a `disabledIntegrations` field that mirrors the CRD 1:1:
79+
80+
```go
81+
type UIConfigResponse struct {
82+
UICanaryDeploymentWeight int `json:"uiCanaryDeploymentWeight"`
83+
WorkatoWebHookURL string `json:"workatoWebHookURL"`
84+
DisabledIntegrations []string `json:"disabledIntegrations"`
85+
}
86+
```
87+
88+
Example JSON response from `GET /api/v1/uiconfig` when OpenShift Lightspeed is down:
89+
90+
```json
91+
{
92+
"uiCanaryDeploymentWeight": 20,
93+
"workatoWebHookURL": "https://...",
94+
"disabledIntegrations": ["openshift-lightspeed"]
95+
}
96+
```
97+
98+
Example when everything is healthy (field not set in CR):
99+
100+
```json
101+
{
102+
"uiCanaryDeploymentWeight": 20,
103+
"workatoWebHookURL": "https://...",
104+
"disabledIntegrations": []
105+
}
106+
```
107+
108+
The handler normalizes nil to an empty slice before serialization, so the response always contains `"disabledIntegrations": []` (never absent or `null`). The UI checks `disabledIntegrations.includes("x")` before rendering each integration entry point.
109+
110+
## Implementation Plan
111+
112+
Changes span **three repositories** in the codeready-toolchain ecosystem:
113+
114+
### Phase 1: API types (`codeready-toolchain/api`)
115+
116+
1. Add `DisabledIntegrations []string` to `RegistrationServiceConfig` in `api/v1alpha1/toolchainconfig_types.go` with `+listType=set` and `+optional` markers.
117+
2. Run code generation (`make generate`) to update DeepCopy methods and CRD manifests.
118+
119+
### Phase 2: Host operator (`codeready-toolchain/host-operator`)
120+
121+
1. Bump the `codeready-toolchain/api` dependency in `go.mod` to pick up the new field, then run `make generate manifests` to regenerate the CRD YAML in `config/crd/bases/`.
122+
123+
### Phase 3: Registration service (`codeready-toolchain/registration-service`)
124+
125+
1. Add a `DisabledIntegrations() []string` accessor method on `RegistrationServiceConfig` in `pkg/configuration/configuration.go`. The accessor returns the `[]string` field directly — nil and empty both mean "nothing disabled," so no default-value wrapper is needed.
126+
2. Extend `UIConfigResponse` in `pkg/controller/uiconfig.go` to include the `DisabledIntegrations` field.
127+
3. Update the `GetHandler` to populate the new field from configuration, normalizing nil to an empty slice.
128+
4. Add unit tests for the updated handler.
129+
130+
### Phase 4: UI (out of scope for this design)
131+
132+
The UI team will consume the `disabledIntegrations` field from the `/api/v1/uiconfig` response and hide any integration present in the array.
133+
134+
## Decisions Summary
135+
136+
| # | Question | Decision |
137+
|---|----------|----------|
138+
| Q1 | Allowlist vs. denylist | Denylist (`disabledIntegrations`) — only list broken integrations |
139+
| Q2 | Field type | Simple `[]string` |
140+
| Q3 | Field placement | Under `spec.host.registrationService` |
141+
| Q4 | Endpoint | Extend existing `GET /api/v1/uiconfig` |
142+
| Q5 | Authentication | Keep JWT-secured (no change) |
143+
| Q6 | CRD field name | `disabledIntegrations` |
144+
| Q7 | REST response property | Mirror CRD — `disabledIntegrations` |
145+
146+
Full decision rationale: [enabled-integrations-questions.md](enabled-integrations-questions.md)

‎docs/proposals/enabled-integrations-questions.md‎

Lines changed: 151 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,151 @@
1+
# Integration Visibility — Design Questions
2+
3+
**Status:** Resolved — all decisions made
4+
**Related:** [Design document](enabled-integrations-design.md)
5+
6+
Each question has options with trade-offs and a recommendation. Go through them one by one to form the design, then update the design document.
7+
8+
---
9+
10+
## Q1: Allowlist vs. denylist approach?
11+
12+
When a cluster admin wants to hide a broken integration, the system needs a mechanism. This affects the operational model and how much config admins need to maintain.
13+
14+
### Option C: Denylist approach (`disabledIntegrations`)
15+
16+
List what is disabled. Everything not in the list is enabled.
17+
18+
- **Pro:** Backward compatible — existing deployments show everything.
19+
- **Pro:** Operationally simpler — the common case (everything healthy) is an empty field; admins only add the 1-2 broken integrations.
20+
- **Pro:** Adding a new integration requires no config change — it's enabled by default.
21+
- **Con:** The UI must check each integration against the disabled list (trivial).
22+
23+
**Decision:** Option C — denylist approach. With an allowlist, admins must enumerate every integration; with a denylist the normal state is empty and only broken integrations are listed.
24+
25+
_Considered and rejected: Option A — allowlist when populated (requires listing all integrations, higher operational overhead), Option B — explicit allowlist with nothing enabled by default (breaking change for existing deployments)._
26+
27+
---
28+
29+
## Q2: Should integrations be simple string identifiers or richer structs?
30+
31+
This determines the shape of the array items in the CRD.
32+
33+
### Option A: Simple `[]string`
34+
35+
```yaml
36+
disabledIntegrations:
37+
- "openshift-lightspeed"
38+
```
39+
40+
- **Pro:** Minimal API surface, easy to understand.
41+
- **Pro:** Follows the pattern used by comma-separated string lists elsewhere in the config (e.g. `domains`, `forbiddenUsernamePrefixes`), but as a proper array.
42+
- **Con:** No room for per-integration metadata (e.g. a display name or URL) without a future API change.
43+
44+
**Decision:** Option A — simple `[]string` keeps the API minimal. The UI already knows how to render each integration; it only needs the enable/disable signal.
45+
46+
_Considered and rejected: Option B — array of structs (over-engineered for current needs, display metadata belongs in UI), Option C — comma-separated string (less idiomatic for CRDs)._
47+
48+
---
49+
50+
## Q3: Where should the field live in the config hierarchy?
51+
52+
The `ToolchainConfig` has a deep nested structure. The field placement affects who "owns" this configuration conceptually.
53+
54+
### Option A: Under `spec.host.registrationService`
55+
56+
```yaml
57+
spec:
58+
host:
59+
registrationService:
60+
disabledIntegrations:
61+
- "openshift-lightspeed"
62+
```
63+
64+
- **Pro:** The registration service is the component that exposes this data; co-locating it with other registration service config is natural.
65+
- **Pro:** Follows the pattern of `uiCanaryDeploymentWeight` and `workatoWebHookURL` which are UI-facing config under `registrationService`.
66+
- **Con:** Conceptually, "which integrations are disabled" could be considered a broader host-level concern, not specific to the registration service.
67+
68+
**Decision:** Option A — co-locate with other UI-facing config under `registrationService`, following existing precedent.
69+
70+
_Considered and rejected: Option B — under `spec.host` directly (only registration service needs it, would bloat HostConfig)._
71+
72+
---
73+
74+
## Q4: Should this be a new endpoint or added to the existing `/api/v1/uiconfig`?
75+
76+
The registration service already has `GET /api/v1/uiconfig` that returns UI-facing configuration (canary weight, Workato URL).
77+
78+
### Option A: Extend `/api/v1/uiconfig`
79+
80+
Add the integration visibility field to the existing `UIConfigResponse`.
81+
82+
- **Pro:** No new endpoint to maintain; the UI already calls `/api/v1/uiconfig`.
83+
- **Pro:** Keeps UI configuration consolidated in one place.
84+
- **Con:** Makes the uiconfig response grow over time; it's currently JWT-secured, which means unauthenticated callers cannot access it.
85+
86+
**Decision:** Option A — add to existing `UIConfigResponse`. Avoids an extra HTTP round-trip and keeps UI configuration consolidated.
87+
88+
_Considered and rejected: Option B — new dedicated endpoint (extra maintenance burden, extra HTTP call from the UI)._
89+
90+
---
91+
92+
## Q5: Should the endpoint be secured (JWT) or unsecured?
93+
94+
Currently `/api/v1/uiconfig` requires JWT authentication. The list of disabled integrations is not sensitive, but access control is a consideration.
95+
96+
### Option A: Keep it secured (current behavior for `/api/v1/uiconfig`)
97+
98+
- **Pro:** No security posture change.
99+
- **Pro:** Only authenticated users see what integrations are available.
100+
- **Con:** The UI must have a valid token before it can determine what to render, which may delay the initial page load or require the UI to handle a two-phase render.
101+
102+
**Decision:** Option A — keep the endpoint JWT-secured. No security posture change needed; the UI already has a token when it renders integration entry points.
103+
104+
_Considered and rejected: Option B — unsecured (would also expose workatoWebHookURL and canary weight), Option C — split into new unsecured endpoint (contradicts Q4 decision)._
105+
106+
---
107+
108+
## Q6: What should the CRD field be named?
109+
110+
Naming affects clarity, consistency with the codebase, and how the field appears in the CRD YAML.
111+
112+
### Option A: `disabledIntegrations`
113+
114+
```yaml
115+
disabledIntegrations:
116+
- "openshift-lightspeed"
117+
```
118+
JSON tag: `disabledIntegrations`
119+
120+
- **Pro:** Directly communicates the denylist semantics — these are the integrations that are turned off.
121+
- **Pro:** Consistent with the naming pattern of the approach (listing what's disabled).
122+
- **Con:** Slightly long.
123+
124+
**Decision:** Option A (`disabledIntegrations`) — unambiguous, directly reflects the denylist semantics.
125+
126+
_Considered and rejected: Option B — `integrations` (ambiguous), Option C — `hiddenIntegrations` (UI term out of place in a CRD)._
127+
128+
---
129+
130+
## Q7: What should the REST response JSON property be named?
131+
132+
The CRD field is `disabledIntegrations` (a denylist), but the REST response could either mirror the CRD directly or translate the semantics for the UI's convenience. This affects where the "is this integration disabled?" logic lives.
133+
134+
### Option A: Mirror the CRD — expose `disabledIntegrations` in the JSON response
135+
136+
```json
137+
{
138+
"uiCanaryDeploymentWeight": 20,
139+
"workatoWebHookURL": "https://...",
140+
"disabledIntegrations": ["openshift-lightspeed"]
141+
}
142+
```
143+
144+
- **Pro:** 1:1 mapping with the CRD — simple, transparent, no translation layer.
145+
- **Pro:** The UI can trivially check `disabledIntegrations.includes("x")` before rendering each integration.
146+
- **Pro:** Empty array `[]` unambiguously means "nothing is disabled" (everything enabled).
147+
- **Con:** The UI works with a negative list, which some may find less intuitive than a positive one.
148+
149+
**Decision:** Option A — mirror the CRD directly. Simple, no master list needed, trivial UI check.
150+
151+
_Considered and rejected: Option B — translate to `enabledIntegrations` (requires backend to maintain a hardcoded master list of all integrations, adds a translation layer that can drift)._

0 commit comments

Comments
 (0)