OCPBUGS-87348: Fix ConfigMapSyncDegraded when cloud.openshift.com is absent from pull-secret#1172
Conversation
…absent from pull-secret
openshift-ci-robot
added
jira/severity-important
|
@fsgreco: This pull request references Jira Issue OCPBUGS-87348, which is invalid:
Comment The bug has been updated to refer to the pull request using the external bug tracker. 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. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: fsgreco The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited) Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (2)
🔗 Linked repositories identifiedCodeRabbit considers these linked repositories for cross-repo context during reviews:
🚧 Files skipped from review as they are similar to previous changes (1)
📜 Recent review details🧰 Additional context used📓 Path-based instructions (7)**/*.go📄 CodeRabbit inference engine (AGENTS.md)
Files:
⚙️ CodeRabbit configuration file
Files:
**/*_test.go📄 CodeRabbit inference engine (AGENTS.md)
Files:
⚙️ CodeRabbit configuration file
Files:
{pkg,cmd}/**/*.go📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**/*.{go,java,py,js,ts,jsx,tsx,cs,cpp,c,rb,php}📄 CodeRabbit inference engine (Custom checks)
Files:
pkg/**/*_test.go📄 CodeRabbit inference engine (.claude/skills/unit-test-review.md)
Files:
**/*.{py,js,ts,go,rs,java,rb,php,kt,swift,cs}⚙️ CodeRabbit configuration file
Files:
**⚙️ CodeRabbit configuration file
Files:
🔀 Multi-repo context openshift/consoleFindings[::openshift/console::] frontend/packages/console-shared/src/hooks/useTelemetry.ts:46-55,54-55
[::openshift/console::] frontend/packages/console-shared/src/hooks/tests/useTelemetry.spec.ts:362,389
[::openshift/console::] frontend/public/components/global-telemetry-notifications.tsx:29-30
[::openshift/console::] frontend/packages/console-dynamic-plugin-sdk/src/api/segment-analytics.ts:6-31
[::openshift/console::] frontend/public/components/utils/service-level.tsx:104-108
[::openshift/console::] README.md:531
Conclusion: The operator change to return ("", nil) for missing cloud.openshift.com and to skip OCM fetch when token is empty will result in ORGANIZATION_ID and ACCOUNT_MAIL being unset/empty in SERVER_FLAGS.telemetry. Frontend consumers read these fields (useTelemetry, tests, telemetry plugins) and already access pull-secret.cloud.openshift.com.auth directly; this change appears consistent with disconnected cluster expectations but will cause those telemetry fields to be empty — tests or UI behavior that expect them may need adjustment. 🔇 Additional comments (4)
WalkthroughThe PR updates telemetry token retrieval to gracefully handle missing credentials and prevents downstream metadata population when no token is available. ChangesTelemetry Token Graceful Handling
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 14 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (14 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
pkg/console/telemetry/telemetry_test.go (1)
6-12: ⚡ Quick winAdd import grouping comments per conventions.
Import organization should include grouping comments as specified in CONVENTIONS.md. Add a
// kubecomment before the k8s.io imports.♻️ Proposed fix
import ( + // standard lib "testing" "time" + // kube corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/tools/cache" corev1listers "k8s.io/client-go/listers/core/v1" + // operator (internal) "github.com/openshift/console-operator/pkg/api" )As per coding guidelines, group imports with comments: standard lib, 3rd party, kube/openshift, internal.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/console/telemetry/telemetry_test.go` around lines 6 - 12, The imports in telemetry_test.go are missing the required grouping comment for Kubernetes imports; update the import block to include grouping comments per CONVENTIONS.md (e.g., add a `// kube` comment immediately before the k8s.io imports that include corev1, metav1, cache, corev1listers) and ensure the overall import groups follow the standard order (standard lib, 3rd party, kube, internal) so github.com/openshift/console-operator/pkg/api remains in the internal group.Source: Coding guidelines
pkg/console/telemetry/telemetry.go (1)
85-85: ⚡ Quick winReplace
fmt.Printlnwith structured logging.Operator code should use
klogfor structured logging instead offmt.Println. This error is already being returned on line 86, so logging here is optional, but if retained it should use klog.♻️ Proposed fix
- fmt.Println("Error decoding JSON:", err) - return "", err + return "", fmt.Errorf("failed to decode .dockerconfigjson: %w", err)Or if you want to keep the log:
- fmt.Println("Error decoding JSON:", err) + klog.V(4).Infof("failed to decode .dockerconfigjson: %v", err) return "", errAs per coding guidelines, use klog for logging in operator code and wrap errors with %w to preserve error chains.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/console/telemetry/telemetry.go` at line 85, Replace the fmt.Println("Error decoding JSON:", err) call with klog structured logging and preserve the error chain on return; specifically, change the println to klog.ErrorS(err, "error decoding JSON") (or klog.Errorf("error decoding JSON: %v", err) if you prefer formatted logging) and when returning the error use fmt.Errorf("error decoding JSON: %w", err) so the original err is wrapped; locate the println in pkg/console/telemetry/telemetry.go where the local variable err is set and update both the log call and the subsequent return to use the wrapped error.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@pkg/console/telemetry/telemetry_test.go`:
- Around line 6-12: The imports in telemetry_test.go are missing the required
grouping comment for Kubernetes imports; update the import block to include
grouping comments per CONVENTIONS.md (e.g., add a `// kube` comment immediately
before the k8s.io imports that include corev1, metav1, cache, corev1listers) and
ensure the overall import groups follow the standard order (standard lib, 3rd
party, kube, internal) so github.com/openshift/console-operator/pkg/api remains
in the internal group.
In `@pkg/console/telemetry/telemetry.go`:
- Line 85: Replace the fmt.Println("Error decoding JSON:", err) call with klog
structured logging and preserve the error chain on return; specifically, change
the println to klog.ErrorS(err, "error decoding JSON") (or klog.Errorf("error
decoding JSON: %v", err) if you prefer formatted logging) and when returning the
error use fmt.Errorf("error decoding JSON: %w", err) so the original err is
wrapped; locate the println in pkg/console/telemetry/telemetry.go where the
local variable err is set and update both the log call and the subsequent return
to use the wrapped error.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: d6e39af5-9614-4a1f-840d-d9334461e292
📒 Files selected for processing (3)
pkg/console/operator/sync_v400.gopkg/console/telemetry/telemetry.gopkg/console/telemetry/telemetry_test.go
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
openshift/console(manual)
📜 Review details
🧰 Additional context used
📓 Path-based instructions (9)
**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
**/*.go: Follow Go coding standards and patterns documented in CONVENTIONS.md
Organize imports according to conventions documented in CONVENTIONS.md
Usegofmtto format Go code with standard formatting
Rungo vetchecks on all Go packagesFollow Go coding standards and patterns as documented in CONVENTIONS.md, including proper import organization
Organize Go code following the repository structure: main entry point in
cmd/console/main.go, API constants inpkg/api/, operator command setup inpkg/cmd/operator/, and version command inpkg/cmd/version/
**/*.go: Usegofmtfor formatting Go code
Follow standard Go naming conventions
Group imports in order: standard lib, 3rd party, kube/openshift, internal (marked with comments)
Use meaningful error messages with context in Go code
Set status conditions usingstatus.Handle*functions with type prefixes (*Degraded, *Progressing, *Available, *Upgradeable)
Use typed errors and wrap errors to preserve stack contextOpenShift Tests Extension (OTE) binaries communicate with openshift-tests via JSON on stdout. Flag non-JSON stdout from the main binary process in process-level code: main(), init(), TestMain(), BeforeSuite(), AfterSuite(), SynchronizedBeforeSuite(), RunSpecs() setup, or top-level var/const initializers. Common violations include klog writing to stdout (must redirect to stderr), fmt.Print/Println/Printf in main or suite setup, and Ginkgo v2 suite configuration emitting warnings to stdout
**/*.go: Imports must be grouped with comments in the order: standard lib, 3rd party, kube, openshift, and operator (internal)
Wrap errors with context usingfmt.Errorf("failed to X: %w", err)pattern to provide meaningful error messages
**/*.go: Do NOT use deprecated ioutil package functions. Use os.ReadFile() instead of ioutil.ReadFile(), os.WriteFile() instead of ioutil.WriteFile(), and io.ReadAll() instead of ioutil.ReadAll() (Go 1.16+)
Do NOT use net.Dial() directly. Use (&net.Dialer{}).DialConte...
Files:
pkg/console/telemetry/telemetry.gopkg/console/operator/sync_v400.gopkg/console/telemetry/telemetry_test.go
⚙️ CodeRabbit configuration file
**/*.go: Review Go code following OpenShift operator patterns.
See CONVENTIONS.md for coding standards and patterns.Refer to the following skills based on CODE PATTERNS, not just file paths:
Refer to /controller-review when code contains:
- Controller struct types (e.g.,
type *Controller struct)func New*Controller(factory functionsfactory.New().WithFilteredEventsInformers(pattern.ToController(method callsSync(ctx context.Context, controllerContext factory.SyncContext)methodsoperatorConfig.Spec.ManagementStatechecksstatus.NewStatusHandlerorstatus.Handle*functionsRefer to /sync-handler-review when code contains:
- Main operator sync functions (e.g.,
sync_v400.gocontent)- Sequential resource syncing with early returns
- Incremental reconciliation loops
- Multiple
resourceapply.Apply*()calls in sequence- Dependency ordering of ConfigMaps → Secrets → Service Accounts → RBAC → Services → Deployments → Routes
- Feature gate conditional logic
Refer to /go-quality-review for all Go code to check:
- Deprecated imports:
ioutil.ReadFile,ioutil.WriteFile,ioutil.ReadAll- Deprecated patterns:
DialwithoutDialContext- Error handling: missing
%win fmt.Errorf- Code smells: deep nesting (4+ levels), functions >100 lines
- Magic values: unexplained numbers/strings
- Context propagation:
context.Background()instead of passed ctx- Missing godoc on exported functions
Files:
pkg/console/telemetry/telemetry.gopkg/console/operator/sync_v400.gopkg/console/telemetry/telemetry_test.go
{pkg,cmd}/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
Use gofmt for code formatting on pkg and cmd directories
{pkg,cmd}/**/*.go: Format code usinggofmt -w ./pkg ./cmd
Rungo vetchecks on all Go packages in ./pkg and ./cmd
Files:
pkg/console/telemetry/telemetry.gopkg/console/operator/sync_v400.gopkg/console/telemetry/telemetry_test.go
**/*.{go,java,py,js,ts,jsx,tsx,cs,cpp,c,rb,php}
📄 CodeRabbit inference engine (Custom checks)
**/*.{go,java,py,js,ts,jsx,tsx,cs,cpp,c,rb,php}: Flag logging that may expose passwords, tokens, API keys, PII (email, SSN, credit card), session IDs, internal hostnames, or customer data
Flag MD5, SHA1, DES, RC4, 3DES, Blowfish, ECB mode usage. Flag custom crypto implementations. Flag non-constant-time comparison of secrets or tokens
Files:
pkg/console/telemetry/telemetry.gopkg/console/operator/sync_v400.gopkg/console/telemetry/telemetry_test.go
**/*.{py,js,ts,go,rs,java,rb,php,kt,swift,cs}
⚙️ CodeRabbit configuration file
**/*.{py,js,ts,go,rs,java,rb,php,kt,swift,cs}: Injection prevention (prodsec-skills):
- SQL: parameterized queries only; no string concatenation
- Command: no shell=True, os.system, or backtick exec with user input
- LDAP/XPath: escape special characters in filters
- Path traversal: canonicalize paths, reject ../
- Deserialization: no pickle/yaml.load()/eval on untrusted data
- Prototype pollution: no recursive merge of untrusted objects
- Validate at trust boundaries with allow-lists, not deny-lists
- Normalize Unicode and anchor regexes (^$); watch for ReDoS
Files:
pkg/console/telemetry/telemetry.gopkg/console/operator/sync_v400.gopkg/console/telemetry/telemetry_test.go
**
⚙️ CodeRabbit configuration file
**: # OpenShift Console Operator - AI Context HubThis file serves as the central AI documentation hub for the OpenShift Console Operator project. AI assistants (Claude, Cursor, Copilot, CodeRabbit, etc.) use this and the linked documents to understand project context.
Go Version and Dependencies
Go Version and Dependencies
- Go version: 1.24.0 (toolchain: go1.24.4)
- Dependency management: Uses
go.modwith vendoring- Build flags: Use
GOFLAGS="-mod=vendor"for builds and tests to ensure vendored dependencies are used- Key dependencies: openshift/api, openshift/library-go, k8s.io client libraries
- Go version: 1.24.0 (toolchain: go1.24.4)
- Dependency management: Uses
go.modwith vendoring- Build flags: Use
GOFLAGS="-mod=vendor"for builds and tests to ensure vendored dependencies are used- Key dependencies: openshift/api, openshift/library-go, k8s.io client libraries
Quick Reference
This Repository
Document Purpose ARCHITECTURE.md System architecture, components, repository structure CONVENTIONS.md Go coding standards, patterns, import organization TESTING.md Testing patterns, commands, debugging README.md Project README with setup instructions Console Repository (openshift/console)
For frontend-related guidelines, see the openshift/console repository:
Document Purpose STYLEGUIDE.md Frontend code style guidelines INTERNATIONALIZATION.md i18n patterns and translation guidelines CONTRIBUTING.md Contribution guidelines for the console project Project Summary
The **console-operator...
Files:
pkg/console/telemetry/telemetry.gopkg/console/operator/sync_v400.gopkg/console/telemetry/telemetry_test.go
**/*sync*.go
📄 CodeRabbit inference engine (CONVENTIONS.md)
Implement sync loops (
sync_v400) incrementally: start from zero, create/update missing requirements, and return to continue on next loop
**/*sync*.go: Sync handler implementations must use incremental sync pattern where each reconciliation loop returns early on error rather than collecting errors and continuing
Sync resources in dependency order: ConfigMaps/Secrets → Service Accounts → RBAC (Roles, RoleBindings) → Services → Deployments → Routes
Return early on errors to maintain incremental sync behavior instead of continuing execution or logging and proceeding
Use resourceapply.Apply*() functions to handle both resource creation and update operations
Ensure deleted resources are cleaned up from the cluster when removed from config, with proper handling for NotFound errors
Check feature gates before syncing gated resources to ensure conditional resource reconciliation
Status updates must reflect actual reconciliation state by adding conditions as operations complete using status handlers with HandleProgressingOrDegraded and HandleAvailable
Respect ManagementState configurations and implement cleanup logic for Removed state
Avoid mutating live objects; instead build desired state separately before applying
Do not sync all resources regardless of errors; stop reconciliation on dependency failures
Files:
pkg/console/operator/sync_v400.go
**/{deploy,config,operator,controllers}/**/*.{yaml,yml,go}
📄 CodeRabbit inference engine (Custom checks)
**/{deploy,config,operator,controllers}/**/*.{yaml,yml,go}: When deployment manifests, operator code, or controllers are added or modified, check for scheduling constraints that assume a standard HA topology without topology-awareness. Flag required pod anti-affinity (requiredDuringSchedulingIgnoredDuringExecution with topologyKey: kubernetes.io/hostname) that breaks on SNO, TNF with replicas > 2, and TNA
When deployment manifests, operator code, or controllers are added or modified, check for pod topology spread constraints with whenUnsatisfiable: DoNotSchedule and hostname topology key that breaks on SNO and is problematic when maxSkew exceeds the number of schedulable nodes on TNF and TNA
When deployment manifests, operator code, or controllers are added or modified, check for replica count derived from node count without considering SNO, TNF, and TNA topology variations where fewer than 3 control-plane nodes exist, and TNA's arbiter node should not run general workloads
When deployment manifests, operator code, or controllers are added or modified, check for nodeSelector or node affinity targeting control-plane nodes (node-role.kubernetes.io/master or node-role.kubernetes.io/control-plane) that will fail on HyperShift where no nodes carry these labels
When deployment manifests, operator code, or controllers are added or modified, check for scheduling workloads to all control-plane nodes equally without excluding arbiter nodes. Flag broad or wildcard tolerations that inadvertently schedule to resource-constrained arbiter nodes on TNA clusters
When deployment manifests, operator code, or controllers are added or modified, check for assumptions that dedicated worker nodes exist. Code targeting only worker nodes via node-role.kubernetes.io/worker nodeSelector may need to also consider nodes that carry both control-plane and worker roles on SNO and TNF
When deployment manifests, operator code, or controllers are added or modified, check PodDisruptionBudgets designe...
Files:
pkg/console/operator/sync_v400.go
**/*_test.go
📄 CodeRabbit inference engine (AGENTS.md)
Follow testing patterns and commands documented in TESTING.md
Follow testing patterns and commands as documented in TESTING.md, including running unit tests with 'make test-unit' and checks with 'make check'
**/*_test.go: Use table-driven tests for comprehensive coverage
Usehttptestfor HTTP handler testing in Go
Include proper cleanup functions in tests
Test both success and failure pathsCheck and handle all errors in tests using t.Fatalf() or t.Errorf(); do not ignore errors with blank identifiers
Files:
pkg/console/telemetry/telemetry_test.go
⚙️ CodeRabbit configuration file
**/*_test.go: Review test code for quality and patterns.Refer to /unit-test-review when test is in pkg//*_test.go:**
- Table-driven test structure with test cases
- Use of
go-test/deepfor struct comparisons- Test naming conventions (TestFunctionName)
- Error handling with
wantErrpattern- Edge case coverage (nil, empty, boundary values)
- Proper assertions with helpful error messages
- Test isolation (no shared mutable state)
Refer to /e2e-test-review when test contains:
framework.MustNewClientset(t, nil)or similar e2e framework usagewait.Pollorwait.PollImmediatepatternsretry.RetryOnConflictfor updates- Cleanup via
deferfunctions- Console/operator CR manipulations
- Test assertions on cluster state
Suggest to use /e2e-test-review when:
- PR adds new feature requiring e2e coverage
- Test file is empty or skeleton
- Comments indicate "TODO: add test"
Review for common issues:
- Missing cleanup (defer statements)
- Using
time.Sleepinstead ofwait.Poll- Missing context timeouts
- Vague error messages in assertions
- Tests without table-driven structure when testing multiple cases
- Ignoring errors with
_- Tests without assertions
Files:
pkg/console/telemetry/telemetry_test.go
pkg/**/*_test.go
📄 CodeRabbit inference engine (.claude/skills/unit-test-review.md)
pkg/**/*_test.go: Go unit tests in pkg/**/*_test.go files should use table-driven test pattern for clarity and completeness with struct containing test cases
Test function names should be descriptive and clearly indicate what is being tested (e.g., TestGetNodeComputeEnvironments, TestNewRouteConfig)
Test case names within table-driven tests should explain the scenario being tested (e.g., 'Custom hostname and TLS secret set'), not be vague (e.g., 'test1')
Use github.com/go-test/deep package for struct comparisons to show exact differences instead of simple equality checks or manual field-by-field comparisons
Test both success and failure paths, including edge cases such as empty inputs (nil, "", empty slices/maps), boundary values (0, -1, max int), missing labels/fields, duplicate values, and large inputs
Organize test code using Arrange-Act-Assert pattern: setup test data in Arrange phase, call the function in Act phase, verify results in Assert phase
Check error presence using pattern (err != nil) != tt.wantErr instead of ignoring errors with underscore or missing error checks
When checking specific error messages, use strings.Contains to verify error content instead of just checking error presence
Extract common test setup into helper functions rather than duplicating setup code across multiple tests
Write specific assertion error messages with context (e.g., 'expected %d nodes, got %d') rather than vague messages (e.g., 'failed')
Inline simple test data in test structs; extract complex fixtures into testdata files or helper functions
Avoid red flags in tests: no table-driven tests for multi-scenario functions, tests without subtests, tests depending on execution order, global mutable state, hardcoded sleeps, tests without assertions, ignoring errors with underscore, testing implementation details instead of behavior, or overly complex test setup
Files:
pkg/console/telemetry/telemetry_test.go
🔀 Multi-repo context openshift/console
[::openshift/console::frontend/packages/console-shared/src/hooks/useTelemetry.ts:54]
- Reads telemetry fields from window.SERVER_FLAGS.telemetry (ORGANIZATION_ID, ACCOUNT_MAIL). These fields may be populated by the operator/telemetry code now short-circuiting when access token is empty.
[::openshift/console::frontend/packages/console-shared/src/hooks/useTelemetry.ts:55]
- Uses ACCOUNT_MAIL to compute accountMailDomain.
[::openshift/console::frontend/packages/console-shared/src/hooks/tests/useTelemetry.spec.ts:362]
- Test references ACCOUNT_MAIL.
[::openshift/console::frontend/packages/console-telemetry-plugin/src/components/TelemetryConfiguration.tsx:119]
- Telemetry configuration UI component exists in the console telemetry plugin (user-facing surface that depends on telemetry configuration).
[::openshift/console::frontend/public/components/utils/service-level.tsx:108]
- Accesses secret.auths['cloud.openshift.com'].auth — frontend code expects the pull-secret to potentially contain cloud.openshift.com auth.
[::openshift/console::pkg/console/telemetry/telemetry.go]
- GetAccessToken now returns ("", nil) when cloud.openshift.com entry is missing. This change prevents an error in disconnected clusters.
[::openshift/console::pkg/console/operator/sync_v400.go]
- GetTelemetryConfiguration short-circuits and skips fetching ORGANIZATION_ID and ACCOUNT_MAIL when access token is empty.
Summary impact:
- Operator changes are localized, but frontend reads ORGANIZATION_ID and ACCOUNT_MAIL from SERVER_FLAGS.telemetry; if operator now leaves those unset in disconnected clusters, the frontend will simply see missing/empty values (tests and UI already reference these fields). Additionally, frontend code that directly reads pull-secret auth for cloud.openshift.com exists and may encounter absent entry; the operator change aligns with treating that absence as non-error.
🔇 Additional comments (4)
pkg/console/telemetry/telemetry.go (1)
88-92: LGTM!pkg/console/telemetry/telemetry_test.go (2)
28-37: LGTM!
39-69: LGTM!pkg/console/operator/sync_v400.go (1)
484-486: LGTM!
Assisted-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
fsgreco
force-pushed
the
OCPBUGS-87348--main--fix-ConfigMapSyncDegraded
branch
from
ba28ee2 to
34e37da
Compare
Assisted-by: Claude Sonnet 4.6 <noreply@anthropic.com>
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
/retest |
|
/retest ci/prow/e2e-aws-console failed is not related to this pr changes |
|
/retest @jhadvig is |
|
@fsgreco: 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. |
|
/verified by CI |
openshift-ci-robot
added
the
verified
|
@fsgreco: This PR has been marked as verified by 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. |
2 participants
Analysis / Root cause:
Regression of OCPBUGS-33021.
GetAccessTokentreats a missingcloud.openshift.comkey in the pull-secret as an error, degrading the operator withFailedGetTelemetryConfig. In disconnected clusters this key is intentionally absent.Solution description:
pkg/console/telemetry/telemetry.go: Return("", nil)instead of an error whencloud.openshift.comis missing.pkg/console/operator/sync_v400.go: Skip OCM fetch when access token is empty.pkg/console/telemetry/telemetry_test.go: Two unit tests forGetAccessTokencovering both missing and present key.Test setup:
Unit tests or disconnected cluster with
cloud.openshift.comremoved from pull-secret.Test cases:
TestGetAccessToken_MissingCloudEntryTestGetAccessToken_PresentCloudEntryBrowser conformance:
N/A
Additional info:
Summary by CodeRabbit
Bug Fixes
Tests