Skip to content

Conversation

@schultzp2020
Copy link
Contributor

Signed-off-by: Paul Schultz [email protected]

Description

Adds rhdh-local as a local testing option.

Which issue(s) does this PR fix

  • Fixes #?

PR acceptance criteria

Please make sure that the following steps are complete:

  • GitHub Actions are completed and successful
  • Unit Tests are updated and passing
  • E2E Tests are updated and passing
  • Documentation is updated if necessary (requirement for new features)
  • Add a screenshot if the change is UX/UI related

How to test changes / Special notes to the reviewer

@openshift-ci
Copy link

openshift-ci bot commented Dec 12, 2025

[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 durandom 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

@sonarqubecloud
Copy link

@gustavolira
Copy link
Member

/review

@rhdh-qodo-merge
Copy link

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🔒 Security concerns

Sensitive information handling:
The local YAML configs include an in-code session secret (super-secret-session-key-for-local-testing) and enable dangerouslyDisableDefaultAuthPolicy. While appropriate for local testing, ensure these files are not used in non-local environments and are clearly marked as insecure defaults. No other immediate vulnerabilities (like credential leakage) are evident since defaults.env uses empty placeholders.

⚡ Recommended focus areas for review

Port/Health Check Robustness

The readiness check assumes RHDH is available at http://localhost:7007 and considers HTTP 200/401 as ready. If compose maps a different port/name or service starts slower, this may fail. Consider making port configurable and optionally probing a specific health endpoint with a longer/backoff strategy.

HEALTH_URL="http://localhost:7007"
MAX_WAIT=180
ELAPSED=0

# Give the container a moment to start the app
sleep 10

while [ $ELAPSED -lt $MAX_WAIT ]; do
  # Try curl with explicit timeout and handle Windows/Git Bash quirks
  HTTP_CODE=$(curl --connect-timeout 5 --max-time 10 -s -o /dev/null -w "%{http_code}" "$HEALTH_URL" 2>/dev/null)
  CURL_EXIT=$?

  # curl exit code 0 means success, check HTTP code
  if [ $CURL_EXIT -eq 0 ]; then
    if [ "$HTTP_CODE" = "200" ] || [ "$HTTP_CODE" = "401" ]; then
      log_success "RHDH is ready! (HTTP $HTTP_CODE after ${ELAPSED}s)"
      return 0
    fi
  fi

  sleep 5
  ELAPSED=$((ELAPSED + 5))
  log_info "Waiting for RHDH... (${ELAPSED}s elapsed, HTTP: $HTTP_CODE, curl exit: $CURL_EXIT)"
done

log_error "RHDH failed to start within ${MAX_WAIT}s"
log_info "Container logs:"
$CONTAINER_RUNTIME logs rhdh 2>&1 | tail -50
exit 1
Compose Service Name Assumption

Logs are fetched from a container named rhdh, which assumes the compose service uses that name across podman/docker. If rhdh-local changes service names, logs and down commands may not target the correct service. Validate service name or parameterize it.

  log_error "RHDH failed to start within ${MAX_WAIT}s"
  log_info "Container logs:"
  $CONTAINER_RUNTIME logs rhdh 2>&1 | tail -50
  exit 1
}

# Stop rhdh-local
stop_rhdh_local() {
  log_info "Stopping rhdh-local..."
  cd "$RHDH_LOCAL_PATH"
  $CONTAINER_RUNTIME compose down --volumes 2>/dev/null || true
}
📚 Focus areas based on broader codebase context

Test Mode Parity

The local runner doesn't expose a way to select standardized test modes (e.g., "rhdh-latest") used in existing E2E flows. Consider adding a flag to mirror established modes so local runs align with documented scenarios and CI expectations. (Ref 8)

log_info "Starting rhdh-local with image: $RHDH_IMAGE"

cd "$RHDH_LOCAL_PATH"
RHDH_IMAGE="$RHDH_IMAGE" $CONTAINER_RUNTIME compose up -d

# Wait for RHDH to be ready
log_info "Waiting for RHDH to be ready..."
HEALTH_URL="http://localhost:7007"
MAX_WAIT=180
ELAPSED=0

# Give the container a moment to start the app
sleep 10

while [ $ELAPSED -lt $MAX_WAIT ]; do
  # Try curl with explicit timeout and handle Windows/Git Bash quirks
  HTTP_CODE=$(curl --connect-timeout 5 --max-time 10 -s -o /dev/null -w "%{http_code}" "$HEALTH_URL" 2>/dev/null)
  CURL_EXIT=$?

  # curl exit code 0 means success, check HTTP code
  if [ $CURL_EXIT -eq 0 ]; then
    if [ "$HTTP_CODE" = "200" ] || [ "$HTTP_CODE" = "401" ]; then
      log_success "RHDH is ready! (HTTP $HTTP_CODE after ${ELAPSED}s)"
      return 0
    fi
  fi

  sleep 5
  ELAPSED=$((ELAPSED + 5))
  log_info "Waiting for RHDH... (${ELAPSED}s elapsed, HTTP: $HTTP_CODE, curl exit: $CURL_EXIT)"
done

log_error "RHDH failed to start within ${MAX_WAIT}s"
log_info "Container logs:"
$CONTAINER_RUNTIME logs rhdh 2>&1 | tail -50
exit 1

Reference reasoning: The referenced E2E docs demonstrate invoking tests with a specific mode variable for consistent scenarios. Aligning the local script to accept and forward a similar mode improves parity with established testing patterns and reduces divergence between local and CI behavior.

📄 References
  1. redhat-developer/rhdh-operator/tests/e2e/README.md [1-12]
  2. redhat-developer/rhdh-operator/tests/e2e/README.md [67-74]
  3. redhat-developer/rhdh-operator/tests/e2e/README.md [75-83]
  4. redhat-developer/rhdh-operator/tests/e2e/README.md [84-92]
  5. redhat-developer/rhdh-operator/tests/e2e/README.md [93-101]
  6. redhat-developer/rhdh-operator/tests/e2e/README.md [39-50]
  7. redhat-developer/rhdh-operator/tests/e2e/README.md [112-122]
  8. redhat-developer/rhdh-operator/tests/e2e/README.md [123-125]

@github-actions
Copy link
Contributor

@github-actions
Copy link
Contributor

This PR is stale because it has been open 7 days with no activity. Remove stale label or comment or this will be closed in 21 days.

@github-actions github-actions bot added the Stale label Dec 20, 2025
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.

2 participants