Skip to content

ci: enhance Kubernetes tests and add safe mode for fork PRs#961

Open
SB2318 wants to merge 1 commit intoTracer-Cloud:mainfrom
SB2318:feat-ci-stability
Open

ci: enhance Kubernetes tests and add safe mode for fork PRs#961
SB2318 wants to merge 1 commit intoTracer-Cloud:mainfrom
SB2318:feat-ci-stability

Conversation

@SB2318
Copy link
Copy Markdown
Contributor

@SB2318 SB2318 commented Apr 26, 2026

Fixes #926

Describe the changes you have made in this PR -

Fixed CI formatting issues by aligning code with ruff format expectations
Ensured all lint and format checks pass successfully in CI
Stabilized CI pipeline by introducing safe vs full execution logic
Skipped Kubernetes and AWS-dependent tests for forked PRs (no secrets environment)
Prevented CI failures caused by missing external credentials
Preserved full test execution for trusted environments (main branch / non-fork PRs)
Improved overall CI reliability without altering core application behavior

Demo/Screenshot for feature changes and bug fixes -

Image

Code Understanding and AI Usage

Did you use AI assistance (ChatGPT, Claude, Copilot, etc.) to write any part of this code?

  • No, I wrote all the code myself
  • Yes, I used AI assistance (continue below)

If you used AI assistance:

  • I have reviewed every single line of the AI-generated code
  • I can explain the purpose and logic of each function/component I added
  • I have tested edge cases and understand how the code handles them
  • I have modified the AI output to follow this project's coding standards and conventions

Explain your implementation approach:


Checklist before requesting a review

  • I have added proper PR title and linked to the issue
  • I have performed a self-review of my code
  • I can explain the purpose of every function, class, and logic block I added
  • I understand why my changes work and have tested them thoroughly
  • I have considered potential edge cases and how my code handles them
  • If it is a core feature, I have added thorough tests
  • My code follows the project's style guidelines and conventions

Note: Please check Allow edits from maintainers if you would like us to assist in the PR.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Apr 26, 2026

Greptile Summary

This PR introduces "safe mode" guards across all CI jobs so that fork PRs without access to repository secrets skip credential-dependent tests rather than failing, while trusted pushes to main and non-fork PRs continue to run the full suite. It also adds app/.gitattributes to normalise line endings for the app/ directory.

Confidence Score: 5/5

Safe to merge; all remaining findings are P2 style/quality suggestions that do not affect correctness.

The core logic — skipping secret-dependent tests for fork PRs while preserving full execution for trusted environments — is sound and correct. The two P2 findings (wasteful setup for k8s-titled fork PRs and limited .gitattributes scope) are minor and do not cause CI failures or incorrect behaviour.

No files require special attention for merging.

Important Files Changed

Filename Overview
.github/workflows/ci.yml Adds safe-mode guards for fork PRs across test, test-kubernetes, and test-thorough jobs; adds repo-name guards to prevent cross-fork triggering; minor inconsistency in test-kubernetes job-level condition for PR-title-triggered runs.
app/.gitattributes New file enforcing LF line endings for app/ directory; does not cover tests/ or repository root files.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[CI Triggered] --> B{Event type?}
    B -->|push to main, own repo| C[Full mode: all jobs run with secrets]
    B -->|pull_request| D{Fork PR?}
    D -->|No - trusted PR| E[Full mode: all jobs run with secrets]
    D -->|Yes - fork PR| F[Safe mode]

    F --> G[test job: Run safe tests\n--ignore=e2e --ignore=synthetic]
    F --> H{k8s or kubernetes\nin PR title?}
    H -->|Yes| I[test-kubernetes job triggers\nbut Run Kubernetes tests step skipped]
    H -->|No| J[test-kubernetes job skipped at job level]
    F --> K[test-thorough: skipped\njob-level repo+push guard]

    C --> L[test: full pytest + coverage]
    C --> M[test-kubernetes: full k8s tests with AWS creds]
    C --> N[test-thorough: E2E matrix\nchecks AWS creds present before running]
Loading

Reviews (1): Last reviewed commit: "ci: enhance Kubernetes tests and add saf..." | Re-trigger Greptile

Comment thread .github/workflows/ci.yml
Comment on lines 192 to 195
if: >-
github.event_name == 'push' ||
(github.repository == 'Tracer-Cloud/opensre' && github.event_name == 'push') ||
contains(github.event.pull_request.title, 'k8s') ||
contains(github.event.pull_request.title, 'kubernetes')
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 Fork PRs with "k8s"/"kubernetes" in title still trigger the job

The job-level if condition gates the push leg on github.repository, but the two contains(…title…) legs have no repo guard. A fork PR whose title includes "k8s" or "kubernetes" will pass this filter, spin up the runner, check out, and install all dependencies — only to be stopped at the step level. All that setup work is wasted CI time without benefit.

Suggested change
if: >-
github.event_name == 'push' ||
(github.repository == 'Tracer-Cloud/opensre' && github.event_name == 'push') ||
contains(github.event.pull_request.title, 'k8s') ||
contains(github.event.pull_request.title, 'kubernetes')
if: >-
(github.repository == 'Tracer-Cloud/opensre' && github.event_name == 'push') ||
(github.repository == 'Tracer-Cloud/opensre' && contains(github.event.pull_request.title, 'k8s')) ||
(github.repository == 'Tracer-Cloud/opensre' && contains(github.event.pull_request.title, 'kubernetes'))

Comment thread app/.gitattributes
@@ -0,0 +1 @@
* text=auto eol=lf
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 .gitattributes scope limited to app/ only

Placing this file under app/ means the eol=lf normalisation applies only to files in that subdirectory. The CI lint/format steps also check tests/, which is a sibling directory and is not covered. If CRLF line-ending issues exist in tests/, this file will not fix them. The standard practice is to place .gitattributes at the repository root so it applies everywhere.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] CI instability: ruff formatting + multiple test failures — request 1-day observation window

1 participant