Skip to content

NETOBSERV-2653: TLS topology automation#1541

Merged
Amoghrd merged 1 commit into
netobserv:mainfrom
Amoghrd:TLS-topology
May 27, 2026
Merged

NETOBSERV-2653: TLS topology automation#1541
Amoghrd merged 1 commit into
netobserv:mainfrom
Amoghrd:TLS-topology

Conversation

@Amoghrd
Copy link
Copy Markdown
Member

@Amoghrd Amoghrd commented May 19, 2026

Description

TLS Topology automation based on NETOBSERV-2609

Dependencies

n/a

Results:

     Spec                                              Tests  Passing  Failing  Pending  Skipped
┌────────────────────────────────────────────────────────────────────────────────────────────────┐
│ ✔  tls_dashboards.cy.ts                     03:39        3        3        -        -        - │
└────────────────────────────────────────────────────────────────────────────────────────────────┘
  ✔  All specs passed!                        03:39        3        3        -        -        -

Checklist

  • Does the changes in PR need specific configuration or environment set up for testing?
    • if so please describe it in PR description.
  • I have added thorough unit tests for the change.
  • QE requirements (check 1 from the list):
    • Standard QE validation, with pre-merge tests unless stated otherwise.
    • Regression tests only (e.g. refactoring with no user-facing change).
    • No QE (e.g. trivial change with high reviewer's confidence, or per agreement with the QE team).

Summary by CodeRabbit

  • Tests
    • Added automated test infrastructure for TLS server and client deployment to support comprehensive testing scenarios.
    • Enhanced TLS dashboard test validation, including lock icon rendering and cleartext traffic detection.

Review Change Stack

@openshift-ci-robot
Copy link
Copy Markdown
Collaborator

openshift-ci-robot commented May 19, 2026

@Amoghrd: This pull request references NETOBSERV-2653 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 "5.0.0" version, but no target version was set.

Details

In response to this:

Description

TLS Topology automation based on NETOBSERV-2609

Dependencies

n/a

Checklist

  • Does the changes in PR need specific configuration or environment set up for testing?
    • if so please describe it in PR description.
  • I have added thorough unit tests for the change.
  • QE requirements (check 1 from the list):
  • Standard QE validation, with pre-merge tests unless stated otherwise.
  • Regression tests only (e.g. refactoring with no user-facing change).
  • No QE (e.g. trivial change with high reviewer's confidence, or per agreement with the QE team).

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.

@openshift-ci
Copy link
Copy Markdown

openshift-ci Bot commented May 19, 2026

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign memodi for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 19, 2026

📝 Walkthrough

Walkthrough

Adds a Kubernetes YAML fixture defining a TLS server with NGINX (HTTP/HTTPS) and a client Pod for testing, then updates the Cypress TLS test suite to deploy this fixture, verify TLS lock icon rendering in topology, and validate TLS dashboard panels.

Changes

TLS Test Infrastructure and Dashboard Validation

Layer / File(s) Summary
TLS Test Fixture: Server and Client Environment
web/cypress/fixtures/test-tls-server-client.yaml
Kubernetes YAML fixture with test-tls-server namespace containing an NGINX ConfigMap (HTTP on 8080, HTTPS on 8443 with TLSv1.2/1.3), a Deployment with init container generating self-signed certs and hardened security context, and a Service routing ports. test-tls-client namespace contains a Pod making repeated curl requests over TLS and HTTP.
Cypress Test Suite: Fixture Deployment, Topology Tests, and Dashboard Validation
web/cypress/integration-tests/tls_dashboards.cy.ts
Imports topology filter helpers, deploys and waits for TLS server/client fixture in setup, adds TLS Topology test suite verifying lock icon states and cleartext traffic behavior via UI toggles, updates dashboard verification to cover all TLS panels, and extends cleanup to delete the fixture.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main change: implementing TLS topology automation, which aligns with the changeset of adding TLS test fixtures and updating the TLS dashboard tests.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description check ✅ Passed The PR description includes required sections (Description, Dependencies, Checklist) with QE requirements properly addressed.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Warning

Review ran into problems

🔥 Problems

Linked repositories: Your configuration references 2 linked repositories, but your current plan allows 1. Analyzed netobserv/netobserv-operator, skipped netobserv/flowlogs-pipeline.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov
Copy link
Copy Markdown

codecov Bot commented May 19, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 52.23%. Comparing base (d2f1913) to head (f9cd1bd).
⚠️ Report is 9 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1541   +/-   ##
=======================================
  Coverage   52.23%   52.23%           
=======================================
  Files         233      233           
  Lines       12552    12552           
  Branches     1575     1575           
=======================================
  Hits         6556     6556           
  Misses       5378     5378           
  Partials      618      618           
Flag Coverage Δ
uitests 56.27% <ø> (ø)
unittests 40.92% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@Amoghrd Amoghrd requested review from jpinsonneau and memodi May 20, 2026 16:55
@Amoghrd
Copy link
Copy Markdown
Member Author

Amoghrd commented May 20, 2026

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 20, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 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.

Inline comments:
In `@web/cypress/fixtures/test-tls-server-client.yaml`:
- Around line 126-134: The curl commands inside the while loop in
test-tls-server-client.yaml use the incorrect redirection order `2>&1 >
/dev/null`; update each curl invocation (the three lines calling curl -sk
--tlsv1.3 ..., curl -sk --tlsv1.2 --tls-max 1.2 ..., and curl -s http://... ) to
silence both stdout and stderr by using `> /dev/null 2>&1` (or `&> /dev/null`)
so stderr is not left visible in pod logs.

In `@web/cypress/integration-tests/tls_dashboards.cy.ts`:
- Around line 106-108: Replace the hardcoded cy.wait(1000) after
cy.visit('/monitoring') with a condition-based wait: after calling
cy.visit('/monitoring') (in tls_dashboards.cy.ts) assert either the URL change
via cy.url().should('include', '/monitoring') or wait for a known dashboard
element to appear (e.g., cy.get('<dashboard-selector>').should('be.visible')),
ensuring the test proceeds only once the page has actually loaded instead of
using a fixed timeout.
- Around line 19-24: Remove the redundant hardcoded wait by deleting the
cy.wait(10000) call; rely on the existing oc wait commands invoked via
cy.adminCLI (the "oc wait --for=condition=Available deployment/tls-server -n
test-tls-server --timeout=180s" and "oc wait --for=condition=Ready pod -n
test-tls-client -l app=tls-client --timeout=120s") to block until resources are
ready, ensuring no fixed delays remain in tls_dashboards.cy.ts.
🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 5e274e25-f5af-4d55-9383-f77beed43c48

📥 Commits

Reviewing files that changed from the base of the PR and between d2f1913 and efad970.

📒 Files selected for processing (2)
  • web/cypress/fixtures/test-tls-server-client.yaml
  • web/cypress/integration-tests/tls_dashboards.cy.ts

Comment thread web/cypress/fixtures/test-tls-server-client.yaml
Comment thread web/cypress/integration-tests/tls_dashboards.cy.ts
Comment thread web/cypress/integration-tests/tls_dashboards.cy.ts Outdated
@Amoghrd Amoghrd added the needs-review Tells that the PR needs a review label May 20, 2026
Copy link
Copy Markdown
Member

@memodi memodi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@openshift-ci openshift-ci Bot added the lgtm label May 26, 2026
@Amoghrd Amoghrd removed the needs-review Tells that the PR needs a review label May 27, 2026
@Amoghrd Amoghrd merged commit 6739cf3 into netobserv:main May 27, 2026
14 of 15 checks passed
@Amoghrd
Copy link
Copy Markdown
Member Author

Amoghrd commented May 27, 2026

/cherry-pick main-pf5

@openshift-cherrypick-robot
Copy link
Copy Markdown
Contributor

@Amoghrd: new pull request created: #1552

Details

In response to this:

/cherry-pick main-pf5

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants