CONSOLE-4986: Add Guided Tour capability flag to Console API and vendor it into console-operator #2644
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
openshift-ci-robot
added
the
jira/valid-reference
|
@krishagarwal278: This pull request references CONSOLE-4986 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.22.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
Hello @krishagarwal278! Some important instructions when contributing to openshift/api: |
📝 WalkthroughWalkthroughA new console UI capability 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Cache: Disabled due to data retention organization setting Knowledge base: Disabled due to data retention organization setting ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
🔇 Additional comments (2)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.5.0)Error: build linters: unable to load custom analyzer "kubeapilinter": tools/_output/bin/kube-api-linter.so, plugin: not implemented Comment |
openshift-ci
bot
added
the
size/S
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
operator/v1/types_console.go (1)
287-294: UpdateMaxItemsand comment to reflect three capabilities.The
Capabilitiesfield hasMaxItems=2(Line 290), but there are now three available capabilities. Users won't be able to configure all three simultaneously. Additionally, the comment on Line 287 still references onlyLightspeedButtonandGettingStartedBanner.Consider updating:
MaxItems=2→MaxItems=3- Comment to include
GuidedTourFeatureSuggested fix
// capabilities defines an array of capabilities that can be interacted with in the console UI. // Each capability defines a visual state that can be interacted with the console to render in the UI. - // Available capabilities are LightspeedButton and GettingStartedBanner. + // Available capabilities are LightspeedButton, GettingStartedBanner and GuidedTourFeature. // Each of the available capabilities may appear only once in the list. // +kubebuilder:validation:MinItems=1 - // +kubebuilder:validation:MaxItems=2 + // +kubebuilder:validation:MaxItems=3 // +listType=map // +listMapKey=name // +optional Capabilities []Capability `json:"capabilities,omitempty"`
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
⛔ Files ignored due to path filters (2)
operator/v1/zz_generated.crd-manifests/0000_50_console_01_consoles.crd.yamlis excluded by!**/zz_generated.crd-manifests/*operator/v1/zz_generated.featuregated-crd-manifests/consoles.operator.openshift.io/AAA_ungated.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**
📒 Files selected for processing (3)
openapi/generated_openapi/zz_generated.openapi.gooperator/v1/types_console.gooperator/v1/zz_generated.swagger_doc_generated.go
🔇 Additional comments (4)
operator/v1/types_console.go (2)
110-112: LGTM!The new
GuidedTourFeatureconstant follows the established pattern and naming convention used by the existing capabilities (LightspeedButton,GettingStartedBanner).
140-141: LGTM!The documentation comment and kubebuilder validation enum are correctly updated to include
GuidedTourFeature.operator/v1/zz_generated.swagger_doc_generated.go (1)
211-215: LGTM! The swagger documentation correctly includesGuidedTourFeaturein the available capabilities list, consistent with the source definition intypes_console.go.openapi/generated_openapi/zz_generated.openapi.go (1)
49953-49959: LGTM!The description correctly includes the new
GuidedTourFeaturecapability alongside the existingLightspeedButtonandGettingStartedBannercapabilities. This aligns with the PR objective.Since this is a generated file, please confirm it was regenerated using the project's code generation tooling (e.g.,
make updateor equivalent) rather than manually edited.
operator/v1/types_console.go
Outdated
| GettingStartedBanner ConsoleCapabilityName = "GettingStartedBanner" | ||
|
|
||
| // guidedTourFeature is the name of the 'Guided Tour' feature in console UI. | ||
| GuidedTourFeature ConsoleCapabilityName = "GuidedTourFeature" |
There was a problem hiding this comment.
@JoelSpeed cause its not a single element but rather a walkthrough the console, in which we are highlighting UI elements. I understand what the nameig could be better but I wasnt really sure which name would be abstract enough to capture the nature of the feature, other then Feature.
That said I can imagine we keep it as simple as GuidedTour
There was a problem hiding this comment.
Yeah I was wondering why not to keep it simple as GuidedTour, unless we think there's going to be a clash with that name being that simple in the future?
There was a problem hiding this comment.
+1 for shorten it to GuidedTour
@krishagarwal278 please update the PR
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
operator/v1/types_console.go (1)
285-294: Update the comment andMaxItemsvalidation for the new capability.The
capabilitiesfield has two inconsistencies with the newGuidedTourcapability:
- Line 287: The comment still says "Available capabilities are LightspeedButton and GettingStartedBanner" but should include
GuidedTour.- Line 290:
MaxItems=2should be updated toMaxItems=3since there are now three available capabilities.Proposed fix
// capabilities defines an array of capabilities that can be interacted with in the console UI. // Each capability defines a visual state that can be interacted with the console to render in the UI. - // Available capabilities are LightspeedButton and GettingStartedBanner. + // Available capabilities are LightspeedButton, GettingStartedBanner, and GuidedTour. // Each of the available capabilities may appear only once in the list. // +kubebuilder:validation:MinItems=1 - // +kubebuilder:validation:MaxItems=2 + // +kubebuilder:validation:MaxItems=3 // +listType=map // +listMapKey=name // +optional Capabilities []Capability `json:"capabilities,omitempty"`
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
⛔ Files ignored due to path filters (2)
operator/v1/zz_generated.crd-manifests/0000_50_console_01_consoles.crd.yamlis excluded by!**/zz_generated.crd-manifests/*operator/v1/zz_generated.featuregated-crd-manifests/consoles.operator.openshift.io/AAA_ungated.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**
📒 Files selected for processing (3)
openapi/generated_openapi/zz_generated.openapi.gooperator/v1/types_console.gooperator/v1/zz_generated.swagger_doc_generated.go
🚧 Files skipped from review as they are similar to previous changes (1)
- operator/v1/zz_generated.swagger_doc_generated.go
🔇 Additional comments (3)
openapi/generated_openapi/zz_generated.openapi.go (1)
49953-49959: Verified: all generated artifacts are consistent and generator-driven.GuidedTour is correctly defined in the source enum (operator/v1/types_console.go:112) with kubebuilder validation, and appears consistently across all generated artifacts including swagger docs, CRD manifests, and OpenAPI definitions. No manual drift detected.
operator/v1/types_console.go (2)
110-112: LGTM!The new
GuidedTourconstant follows the existing naming pattern and documentation style for console capabilities.
140-141: LGTM!The
Capability.Namefield comment and kubebuilder validation enum are correctly updated to include the newGuidedTourcapability.
operator/v1/types_console.go
Outdated
|
|
||
| // capabilities defines an array of capabilities that can be interacted with in the console UI. | ||
| // Each capability defines a visual state that can be interacted with the console to render in the UI. | ||
| // Available capabilities are LightspeedButton and GettingStartedBanner. |
There was a problem hiding this comment.
| // Available capabilities are LightspeedButton, GettingStartedBanner, and GuidedTour. |
There was a problem hiding this comment.
yikes 😬
just added 👍🏻
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
operator/v1/types_console.go (1)
287-294:MaxItemsconstraint needs to be updated to 3.With the addition of
GuidedTour, there are now 3 available capabilities, butMaxItems=2will prevent users from configuring all of them simultaneously. This validation constraint should be updated to match the number of available capabilities.🐛 Proposed fix
// capabilities defines an array of capabilities that can be interacted with in the console UI. // Each capability defines a visual state that can be interacted with the console to render in the UI. // Available capabilities are LightspeedButton, GettingStartedBanner, and GuidedTour. // Each of the available capabilities may appear only once in the list. // +kubebuilder:validation:MinItems=1 - // +kubebuilder:validation:MaxItems=2 + // +kubebuilder:validation:MaxItems=3 // +listType=map // +listMapKey=name // +optional Capabilities []Capability `json:"capabilities,omitempty"`
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
📒 Files selected for processing (1)
operator/v1/types_console.go
🔇 Additional comments (2)
operator/v1/types_console.go (2)
110-112: LGTM!The new
GuidedTourcapability constant follows the established naming pattern and comment style of the existing capability constants.
140-141: LGTM!The kubebuilder validation enum and documentation comment are correctly updated to include
GuidedTour.
|
/retest |
1 similar comment
|
/retest |
krishagarwal278
force-pushed
the
CONSOLE-4986
branch
from
d219d15 to
aabd2aa
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @operator/v1/types_console.go:
- Around line 287-290: Update the kubebuilder validation for the capabilities
list so users can select all three capabilities; change the annotation
+kubebuilder:validation:MaxItems=2 to +kubebuilder:validation:MaxItems=3 for the
field that documents "Available capabilities are LightspeedButton,
GettingStartedBanner, and GuidedTour" (the slice definition near that comment)
so the list allows up to three items.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
⛔ Files ignored due to path filters (2)
operator/v1/zz_generated.crd-manifests/0000_50_console_01_consoles.crd.yamlis excluded by!**/zz_generated.crd-manifests/*operator/v1/zz_generated.featuregated-crd-manifests/consoles.operator.openshift.io/AAA_ungated.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**
📒 Files selected for processing (3)
openapi/generated_openapi/zz_generated.openapi.gooperator/v1/types_console.gooperator/v1/zz_generated.swagger_doc_generated.go
🚧 Files skipped from review as they are similar to previous changes (2)
- operator/v1/zz_generated.swagger_doc_generated.go
- openapi/generated_openapi/zz_generated.openapi.go
🔇 Additional comments (2)
operator/v1/types_console.go (2)
110-112: LGTM!The new
GuidedTourconstant follows the established pattern forConsoleCapabilityNameconstants, with appropriate naming convention and comment documentation.
140-141: LGTM!The kubebuilder validation enum and documentation comment are correctly updated to include
GuidedTouralongside the existing capabilities.
|
CodeRabbit has a point here, once that's fixed I think this is good to go |
|
@krishagarwal278 Please squash this down to a single commit and then I'll tag |
openshift-ci
bot
added
the
tide/merge-method-squash
|
/label tide/merge-method-squash |
|
/remove-label tid/merge-method-squash We don't support squash merges on this repo (in fact you're not supposed to use it on any openshift repo), please squash manually |
|
@JoelSpeed: The label(s) `/remove-label tid/merge-method-squash
DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
/remove-label tide/merge-method-squash |
openshift-ci
bot
removed
the
tide/merge-method-squash
krishagarwal278
force-pushed
the
CONSOLE-4986
branch
from
9b4cb2e to
3f61198
Compare
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: JoelSpeed 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 |
openshift-ci
bot
added
the
approved
|
@krishagarwal278: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
|
QE verification: |
5 participants
Summary
The OpenShift Console currently renders the Guided Tour feature unconditionally. This has caused flakiness in Console E2E (Cypress) tests running in CI, as the guided tour can interfere with UI interactions and timing.
To address this and to give cluster administrators explicit control, the Console API should expose a Guided Tour capability flag that allows the feature to be enabled or disabled at the cluster level.
Motivation
Reduce Console CI flakiness caused by Guided Tour overlays
Allow cluster admins to opt in/out of Guided Tours
Provide a consistent capability-based mechanism aligned with other Console features
Enable test environments to explicitly disable Guided Tours
Scope
Introduce a new GuidedTourFeature capability in the Console API
Ensure the capability is configurable and surfaced to the Console
Vendor the API changes into console-operator