SANDBOX-1769 | docs: design of the "disabled integrations" feature#1255
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds two design documents introducing a denylist Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested labels
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
MikelAlejoBR
force-pushed
the
SANDBOX-1769-docs-feature-enabled-integrations-ho
branch
2 times, most recently
from
2b57f1a to
58f28b9
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
docs/proposals/enabled-integrations-design.md (1)
44-47: Document config propagation behavior for incident response.Lines 44-47 describe cache-based loading, but the doc does not define expected propagation timing (or fallback behavior if refresh fails). Since this feature is meant for operational mitigation, add explicit expectations (refresh trigger/interval and stale-config behavior) so operators can rely on it during outages.
As per coding guidelines, “Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.”
Also applies to: 108-108
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/proposals/enabled-integrations-design.md` around lines 44 - 47, Document expected propagation timing and stale-config behavior for the disabledIntegrations flow: state that how often Registration service refreshes ToolchainConfig (the refresh interval or that it reacts to watch events), how commonconfig.LoadLatest is triggered (poll interval or event-driven), what constitutes a successful refresh, and the fallback when refresh fails (e.g., continue serving last-known disabledIntegrations from GET /api/v1/uiconfig, expose a “lastUpdated” timestamp and an “staleAfter” threshold, and define operator actions if data is stale). Reference ToolchainConfig.disabledIntegrations, the Registration service’s use of commonconfig.LoadLatest, the JWT-secured GET /api/v1/uiconfig endpoint, and the UI hide behavior in the doc so operators know timing and recovery expectations.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/proposals/enabled-integrations-design.md`:
- Around line 53-57: The doc currently allows "free-form" integration
identifiers which risks drift and invalid values; update the proposal to list
canonical integration IDs (e.g., "openshift-lightspeed", "devspaces", "workato")
and add a contract section for validation: require backend to validate entries
in disabledIntegrations against the canonical set, return a clear error for
unknown IDs, and define expected behavior (ignore unknown with warning vs.
reject request) and UI display rules for validation errors so both backend and
UI stay in sync.
In `@docs/proposals/enabled-integrations-questions.md`:
- Around line 29-47: Document and implement identifier validation semantics for
the disabledIntegrations []string: declare the canonical source of truth for
allowed IDs (an enum in the CRD schema or a registry/lookup like an integrations
registry), specify where validation runs (CRD OpenAPI schema enum, admission
webhook, or registration-service guardrail in ValidateIntegrationIDs), and
define the runtime behavior for unknown values (reject on apply with a clear
error, or accept with a logged warning and no-op). Update the proposal text to
reference the specific symbols/places to change: the disabledIntegrations field,
the CRD schema enum, the admission webhook handler
(validateAdmissionReview/ValidateIntegrationIDs), or the registration-service
registration guard so reviewers know where validation is enforced and what
client-visible errors/logging will occur.
---
Nitpick comments:
In `@docs/proposals/enabled-integrations-design.md`:
- Around line 44-47: Document expected propagation timing and stale-config
behavior for the disabledIntegrations flow: state that how often Registration
service refreshes ToolchainConfig (the refresh interval or that it reacts to
watch events), how commonconfig.LoadLatest is triggered (poll interval or
event-driven), what constitutes a successful refresh, and the fallback when
refresh fails (e.g., continue serving last-known disabledIntegrations from GET
/api/v1/uiconfig, expose a “lastUpdated” timestamp and an “staleAfter”
threshold, and define operator actions if data is stale). Reference
ToolchainConfig.disabledIntegrations, the Registration service’s use of
commonconfig.LoadLatest, the JWT-secured GET /api/v1/uiconfig endpoint, and the
UI hide behavior in the doc so operators know timing and recovery expectations.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: f7c077c4-4df4-49eb-8109-87a7f6978e51
📒 Files selected for processing (2)
docs/proposals/enabled-integrations-design.mddocs/proposals/enabled-integrations-questions.md
MikelAlejoBR
changed the title
These are the design documents used for the "disabled integrations" feature, which aims at showing or hiding the available integrations in Sandbox depending on the configuration specified in the ToolchainConfig resource. SANDBOX-1769
MikelAlejoBR
force-pushed
the
SANDBOX-1769-docs-feature-enabled-integrations-ho
branch
from
58f28b9 to
f7ada0a
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
docs/proposals/enabled-integrations-design.md (1)
23-23: Optional: Add language identifier to fenced code block.The ASCII diagram would satisfy the markdown linter by adding a language identifier (e.g.,
textor leave it as-is since the diagram is clear without it).📝 Markdown linter fix (optional)
-``` +```text ┌──────────────┐ ┌──────────────────┐ ┌─────────────────────┐🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/proposals/enabled-integrations-design.md` at line 23, The fenced code block containing the ASCII diagram should include a language identifier (e.g., add "text" after the opening ``` of the diagram block) so the markdown linter recognizes it as a plain-text/code fence; update the opening fence in the ASCII diagram block (the fenced code block that begins with ``` and contains the box diagram) to be ```text (or another suitable identifier) and leave the diagram content unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@docs/proposals/enabled-integrations-design.md`:
- Line 23: The fenced code block containing the ASCII diagram should include a
language identifier (e.g., add "text" after the opening ``` of the diagram
block) so the markdown linter recognizes it as a plain-text/code fence; update
the opening fence in the ASCII diagram block (the fenced code block that begins
with ``` and contains the box diagram) to be ```text (or another suitable
identifier) and leave the diagram content unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: d16d747b-50b8-4c63-b4a1-ad1b03571df6
📒 Files selected for processing (2)
docs/proposals/enabled-integrations-design.mddocs/proposals/enabled-integrations-questions.md
|
|
||
| ## Overview | ||
|
|
||
| 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. |
There was a problem hiding this comment.
I don't know what Workato is but I feel like we should develop an integration with them
There was a problem hiding this comment.
Oh, actually it's a thing... Whoops
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: alexeykazakov, MatousJobanek, mfrancisc, MikelAlejoBR The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
coderabbitai
Bot
added
documentation
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/proposals/enabled-integrations-questions.md`:
- Around line 37-88: Remove the Git conflict markers (<<<<<<<, =======, >>>>>>>)
and accept the incoming change (Option A) by keeping only the simple []string
decision for the disabledIntegrations field (i.e., keep the "devspaces" entry
and the Option A prose) and delete the HEAD block that contains the Option
B/enum alternative and duplicated/contradictory lines; ensure the final document
contains the single coherent Decision: Option A and no leftover conflict markers
or duplicated sections.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: faa18ec5-264f-494a-a5b2-5a2084eeaa29
📒 Files selected for processing (2)
docs/proposals/enabled-integrations-design.mddocs/proposals/enabled-integrations-questions.md
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: GolangCI Lint
- GitHub Check: Build & push operator bundles & dashboard image for e2e tests
- GitHub Check: test
- GitHub Check: govulncheck
🧰 Additional context used
📓 Path-based instructions (1)
**
⚙️ CodeRabbit configuration file
-Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.
Files:
docs/proposals/enabled-integrations-questions.mddocs/proposals/enabled-integrations-design.md
🪛 LanguageTool
docs/proposals/enabled-integrations-questions.md
[style] ~168-~168: ‘out of place’ might be wordy. Consider a shorter alternative.
Context: ...ption C — hiddenIntegrations (UI term out of place in a CRD)._ --- ## Q7: What should th...
(EN_WORDINESS_PREMIUM_OUT_OF_PLACE)
🪛 markdownlint-cli2 (0.22.1)
docs/proposals/enabled-integrations-design.md
[warning] 79-79: Heading style
Expected: atx; Actual: setext
(MD003, heading-style)
🔀 Multi-repo context codeready-toolchain/toolchain-common
Linked repositories findings
codeready-toolchain/toolchain-common
-
Test helper exposes a DisabledIntegrations accessor for ToolchainConfig registration service:
- pkg/test/config/toolchainconfig.go: RegistrationServiceOption.DisabledIntegrations(...) — sets config.Spec.Host.RegistrationService.DisabledIntegrations. [::codeready-toolchain/toolchain-common::pkg/test/config/toolchainconfig.go]
-
Configuration/cache logic already reads ToolchainConfig resources (affects where the new field will be stored/loaded):
- pkg/configuration/cache.go — logs and cache handling for ToolchainConfig objects. [::codeready-toolchain/toolchain-common::pkg/configuration/cache.go]
-
Test assertions and utilities reference ToolchainConfig in multiple places (update/tests may need to account for the new field):
- pkg/test/config/toolchainconfig_assertion.go and many test helpers under pkg/test/* reference ToolchainConfig creation/modify utilities. [::codeready-toolchain/toolchain-common::pkg/test/config/toolchainconfig_assertion.go]
🔇 Additional comments (1)
docs/proposals/enabled-integrations-design.md (1)
1-147: Well-structured design document.The design document is comprehensive, clearly articulated, and provides actionable implementation guidance across all affected repositories. The decision to use free-form string identifiers aligns with team feedback, and the denylist approach with safe defaults ensures backward compatibility.
| <<<<<<< HEAD | ||
| - "openshift" | ||
| ======= | ||
| - "devspaces" | ||
| >>>>>>> fd9accf (docs: design of the "disabled integrations" feature) | ||
| ``` | ||
|
|
||
| - **Pro:** Minimal API surface, easy to understand. | ||
| - **Pro:** Follows the pattern used by comma-separated string lists elsewhere in the config (e.g. `domains`, `forbiddenUsernamePrefixes`), but as a proper array. | ||
| <<<<<<< HEAD | ||
| - **Con:** No room for per-integration metadata (e.g. a display name or URL) without a future API change. | ||
| - **Con:** Free-form strings are prone to typos and mismatches between backend and UI — a misspelled identifier is silently ignored. | ||
|
|
||
| ### Option B: Typed enum (`IntegrationName`) with kubebuilder validation | ||
|
|
||
| Define a named string type with a `+kubebuilder:validation:Enum` marker and Go constants for each known integration. The CRD field becomes `[]IntegrationName` instead of `[]string`. | ||
|
|
||
| ```go | ||
| // +kubebuilder:validation:Enum=openshift;openshift-ai;devspaces;ansible-automation-platform;openshift-virtualization | ||
| type IntegrationName string | ||
|
|
||
| const ( | ||
| IntegrationOpenShift IntegrationName = "openshift" | ||
| IntegrationOpenShiftAI IntegrationName = "openshift-ai" | ||
| IntegrationDevSpaces IntegrationName = "devspaces" | ||
| IntegrationAnsibleAutomationPlatform IntegrationName = "ansible-automation-platform" | ||
| IntegrationOpenShiftVirtualization IntegrationName = "openshift-virtualization" | ||
| ) | ||
| ``` | ||
|
|
||
| ```yaml | ||
| disabledIntegrations: | ||
| - "openshift" | ||
| ``` | ||
|
|
||
| - **Pro:** Kubernetes rejects invalid values at admission time — immediate feedback on typos. | ||
| - **Pro:** `kubectl explain` shows the valid values — self-documenting. | ||
| - **Pro:** Go constants provide a canonical reference for backend and UI teams. | ||
| - **Con:** Adding a new integration requires an API module change, CRD regeneration, and rollout. | ||
|
|
||
| **Decision:** Option B — typed enum with kubebuilder validation. The safety of admission-time validation outweighs the cost of requiring an API change for new integrations. That cost is actually desirable — new integrations should be a deliberate, reviewable change. | ||
|
|
||
| _Considered and rejected: Option A — simple `[]string` (too error-prone, no validation), Option C — array of structs (over-engineered for current needs), Option D — comma-separated string (less idiomatic for CRDs)._ | ||
| ======= | ||
| - **Pro:** Does not leak internal product information via hardcoded constants or enum values in the CRD schema. | ||
| - **Con:** No room for per-integration metadata (e.g. a display name or URL) without a future API change. | ||
| - **Con:** Free-form strings are prone to typos — a misspelled identifier is silently ignored. | ||
|
|
||
| **Decision:** Option A — simple `[]string` keeps the API minimal and avoids exposing internal integration details in the CRD schema. The UI ignores identifiers it does not recognize. | ||
|
|
||
| _Considered and rejected: Option B — typed enum with kubebuilder validation (leaks internal product information via hardcoded constants in the CRD), Option C — array of structs (over-engineered for current needs), Option D — comma-separated string (less idiomatic for CRDs)._ | ||
| >>>>>>> fd9accf (docs: design of the "disabled integrations" feature) |
There was a problem hiding this comment.
Resolve the merge conflict before merging.
Lines 37-88 contain unresolved Git merge conflict markers (<<<<<<<, =======, >>>>>>>). This file cannot be committed in this state.
Based on the context:
- The design document (
enabled-integrations-design.md) specifies free-form strings (line 53) - Past review comments indicate the team decided to use simple
[]stringinstead of typed enums (per mfrancisc and Alexey's feedback)
Please resolve the conflict by accepting the incoming change (Option A — simple []string).
🔧 How to resolve
Remove the conflict markers and keep only the incoming version (Option A decision):
### Option A: Simple `[]string`
```yaml
disabledIntegrations:
-<<<<<<< HEAD
- - "openshift"
-=======
- "devspaces"
->>>>>>> fd9accf (docs: design of the "disabled integrations" feature)- Pro: Minimal API surface, easy to understand.
- Pro: Follows the pattern used by comma-separated string lists elsewhere in the config (e.g.
domains,forbiddenUsernamePrefixes), but as a proper array.
-<<<<<<< HEAD
-- Con: No room for per-integration metadata (e.g. a display name or URL) without a future API change.
-- Con: Free-form strings are prone to typos and mismatches between backend and UI — a misspelled identifier is silently ignored.
-### Option B: Typed enum (IntegrationName) with kubebuilder validation
-Define a named string type with a +kubebuilder:validation:Enum marker and Go constants for each known integration. The CRD field becomes []IntegrationName instead of []string.
-```go
-// +kubebuilder:validation:Enum=openshift;openshift-ai;devspaces;ansible-automation-platform;openshift-virtualization
-type IntegrationName string
-const (
- IntegrationOpenShift IntegrationName = "openshift"
- IntegrationOpenShiftAI IntegrationName = "openshift-ai"
- IntegrationDevSpaces IntegrationName = "devspaces"
- IntegrationAnsibleAutomationPlatform IntegrationName = "ansible-automation-platform"
- IntegrationOpenShiftVirtualization IntegrationName = "openshift-virtualization"
-)
-```
-```yaml
-disabledIntegrations:
-
- "openshift"
-```
- "openshift"
-- Pro: Kubernetes rejects invalid values at admission time — immediate feedback on typos.
-- Pro: kubectl explain shows the valid values — self-documenting.
-- Pro: Go constants provide a canonical reference for backend and UI teams.
-- Con: Adding a new integration requires an API module change, CRD regeneration, and rollout.
-Decision: Option B — typed enum with kubebuilder validation. The safety of admission-time validation outweighs the cost of requiring an API change for new integrations. That cost is actually desirable — new integrations should be a deliberate, reviewable change.
-Considered and rejected: Option A — simple []string (too error-prone, no validation), Option C — array of structs (over-engineered for current needs), Option D — comma-separated string (less idiomatic for CRDs).
-=======
+- Pro: Does not leak internal product information via hardcoded constants or enum values in the CRD schema.
- Pro: Does not leak internal product information via hardcoded constants or enum values in the CRD schema.
- Con: No room for per-integration metadata (e.g. a display name or URL) without a future API change.
- Con: Free-form strings are prone to typos — a misspelled identifier is silently ignored.
Decision: Option A — simple []string keeps the API minimal and avoids exposing internal integration details in the CRD schema. The UI ignores identifiers it does not recognize.
Considered and rejected: Option B — typed enum with kubebuilder validation (leaks internal product information via hardcoded constants in the CRD), Option C — array of structs (over-engineered for current needs), Option D — comma-separated string (less idiomatic for CRDs).
->>>>>>> fd9accf (docs: design of the "disabled integrations" feature)
</details>
<details>
<summary>🤖 Prompt for AI Agents</summary>
Verify each finding against the current code and only fix it if needed.
In @docs/proposals/enabled-integrations-questions.md around lines 37 - 88,
Remove the Git conflict markers (<<<<<<<, =======, >>>>>>>) and accept the
incoming change (Option A) by keeping only the simple []string decision for the
disabledIntegrations field (i.e., keep the "devspaces" entry and the Option A
prose) and delete the HEAD block that contains the Option B/enum alternative and
duplicated/contradictory lines; ensure the final document contains the single
coherent Decision: Option A and no leftover conflict markers or duplicated
sections.
</details>
<!-- fingerprinting:phantom:triton:puma:2a1fe1e0-9025-402e-87ec-b9ca9c305a45 -->
<!-- 4e71b3a2 -->
<!-- This is an auto-generated comment by CodeRabbit -->
These are the design documents used for the "disabled integrations" feature, which aims at showing or hiding the available integrations in Sandbox depending on the configuration specified in the ToolchainConfig resource. SANDBOX-1769
MikelAlejoBR
force-pushed
the
SANDBOX-1769-docs-feature-enabled-integrations-ho
branch
from
3985475 to
e7f9b7c
Compare
coderabbitai
Bot
removed
documentation
There was a problem hiding this comment.
♻️ Duplicate comments (1)
docs/proposals/enabled-integrations-questions.md (1)
49-49:⚠️ Potential issue | 🔴 CriticalResolve leftover Git conflict marker in the document.
>>>>>>> fd9accf ...at Line 49 indicates an unresolved merge conflict and must be removed before merge.Suggested fix
-_Considered and rejected: Option B — typed enum with kubebuilder validation (leaks internal product information via hardcoded constants in the CRD), Option C — array of structs (over-engineered for current needs), Option D — comma-separated string (less idiomatic for CRDs)._ ->>>>>>> fd9accf (docs: design of the "disabled integrations" feature) +_Considered and rejected: Option B — typed enum with kubebuilder validation (leaks internal product information via hardcoded constants in the CRD), Option C — array of structs (over-engineered for current needs), Option D — comma-separated string (less idiomatic for CRDs)._As per coding guidelines, “Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/proposals/enabled-integrations-questions.md` at line 49, Remove the leftover Git conflict marker `>>>>>>> fd9accf` from the document (it's an unresolved merge artifact) and ensure any corresponding conflict markers (`<<<<<<<` and `=======`) are resolved or removed so the content is coherent; check the surrounding paragraph for intended text and restore/merge the correct final wording for the section once markers are removed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@docs/proposals/enabled-integrations-questions.md`:
- Line 49: Remove the leftover Git conflict marker `>>>>>>> fd9accf` from the
document (it's an unresolved merge artifact) and ensure any corresponding
conflict markers (`<<<<<<<` and `=======`) are resolved or removed so the
content is coherent; check the surrounding paragraph for intended text and
restore/merge the correct final wording for the section once markers are
removed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 2e5a056b-2f60-46ba-a1e2-d6a6d0fbb89a
📒 Files selected for processing (2)
docs/proposals/enabled-integrations-design.mddocs/proposals/enabled-integrations-questions.md
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: GolangCI Lint
- GitHub Check: Build & push operator bundles & dashboard image for e2e tests
- GitHub Check: test
🧰 Additional context used
📓 Path-based instructions (1)
**
⚙️ CodeRabbit configuration file
-Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.
Files:
docs/proposals/enabled-integrations-design.mddocs/proposals/enabled-integrations-questions.md
🪛 LanguageTool
docs/proposals/enabled-integrations-questions.md
[style] ~129-~129: ‘out of place’ might be wordy. Consider a shorter alternative.
Context: ...ption C — hiddenIntegrations (UI term out of place in a CRD)._ --- ## Q7: What should th...
(EN_WORDINESS_PREMIUM_OUT_OF_PLACE)
🪛 markdownlint-cli2 (0.22.1)
docs/proposals/enabled-integrations-design.md
[warning] 23-23: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔀 Multi-repo context codeready-toolchain/toolchain-common
codeready-toolchain/toolchain-common
-
Test helper exposes DisabledIntegrations setter used to construct ToolchainConfig test objects:
- pkg/test/config/toolchainconfig.go: RegistrationServiceOption.DisabledIntegrations(...) sets config.Spec.Host.RegistrationService.DisabledIntegrations = slices.Clone(values) [::codeready-toolchain/toolchain-common::pkg/test/config/toolchainconfig.go]
-
Multiple test helpers/assertions reference ToolchainConfig and may need to include/expect the new field in fixtures or assertions:
- pkg/test/config/toolchainconfig.go (creation/modification helpers) and pkg/test/config/toolchainconfig_assertion.go (ToolchainConfigAssertion) [::codeready-toolchain/toolchain-common::pkg/test/config/toolchainconfig.go]
- pkg/configuration/test.go uses these helpers to create/update configs in tests [::codeready-toolchain/toolchain-common::pkg/configuration/test.go]
- Many cache unit tests reference ToolchainConfig flows: pkg/configuration/cache_test.go (uses NewToolchainConfigObjWithReset, LoadLatest/GetConfig) [::codeready-toolchain/toolchain-common::pkg/configuration/cache_test.go]
-
Configuration cache reads and stores the ToolchainConfig object; consumers retrieving cached config may observe the new field once CRD/API changes are applied:
- pkg/configuration/cache.go implements LoadLatest, GetConfig, GetCachedConfig and UpdateConfig which will surface the new DisabledIntegrations field when present in the ToolchainConfig resource [::codeready-toolchain/toolchain-common::pkg/configuration/cache.go]
Summary: toolchain-common already includes test helpers and cache logic that touch ToolchainConfig/RegistrationService; these should be kept in sync with the documented CRD field (disabledIntegrations) and tests/fixtures may need updates to assert presence/normalization (empty array vs nil).
🔇 Additional comments (1)
docs/proposals/enabled-integrations-design.md (1)
23-42:⚠️ Potential issue | 🟡 MinorAdd a language tag to the architecture fenced block.
The code fence starting at Line 23 has no language identifier, which triggers MD040 and may fail docs lint pipelines.
Suggested fix
-``` +```text ┌──────────────┐ ┌──────────────────┐ ┌─────────────────────┐ │ Cluster │ │ Registration │ │ Sandbox UI │ ... (loaded via commonconfig.LoadLatest) -``` +```As per coding guidelines, “Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.”
> Likely an incorrect or invalid review comment.
|
MikelAlejoBR
merged commit cd31e36
into
codeready-toolchain:master
4 participants
These are the design documents used for the "disabled integrations" feature, which aims at showing or hiding the available integrations in Sandbox depending on the configuration specified in the ToolchainConfig resource.
Related PRs
Jira ticket
[SANDBOX-1769]
Summary by CodeRabbit