Skip to content

Style: Pass 'make lint'#353

Merged
esnible merged 1 commit intokagenti:mainfrom
esnible:pass-make-lint
Apr 29, 2026
Merged

Style: Pass 'make lint'#353
esnible merged 1 commit intokagenti:mainfrom
esnible:pass-make-lint

Conversation

@esnible
Copy link
Copy Markdown
Contributor

@esnible esnible commented Apr 28, 2026

Summary

Although we have a 'lint' target in our Makefile, it isn't run as part of CI and thus the code has drifted.

This PR allows lint to pass.

We should consider adding make lint to CI or replacing that target with something else -- we don't want this to happen again.

Related issue(s)

Resolves #348

(Optional) Testing Instructions

Type make lint and verify success and no files changed.

Signed-off-by: Ed Snible <snible@us.ibm.com>
@rubambiza
Copy link
Copy Markdown
Contributor

PR Review: Style: Pass 'make lint'

Summary

This PR applies automated formatting and linting fixes across 20 files to make make lint pass cleanly. It resolves #348. The changes are purely stylistic/formatting:

  • Adding missing newlines at end of files (Dockerfiles, YAML, requirements.txt, example deployments)
  • Removing trailing whitespace in YAML, Markdown, and shell scripts
  • Python formatting via ruff/black: consistent double-quote preference, trailing commas, import sorting (isort), line-length compliance, removing unused imports
  • Fixing f-string quoting for cross-version compatibility (e.g., client_payload["clientId"] inside double-quoted f-strings changed to client_payload['clientId'])
  • Removing f-string prefix from string literals without interpolation (f"..." to "...")

Single commit, DCO signed-off, clean authorship. The commit message is Pass 'make lint'.


Findings

Critical (1)

  1. CodeQL: Clear-text logging of sensitive data (HIGH) -- The CodeQL check is failing due to a pre-existing issue in authbridge/authproxy/quickstart/setup_keycloak.py at line 191:

    print(f"export CLIENT_SECRET={secret}")

    This logs a client secret to stdout. While this is a quickstart/demo script and the behavior is intentional (it tells the user what to export), CodeQL flags it as a high-severity finding. This is NOT introduced by this PR -- the code was already there, and the lint PR only reformatted surrounding lines. However, the CodeQL check is currently failing on this PR.

    Recommendation: This can be addressed separately. Consider suppressing the alert with a # nosec / CodeQL annotation on that line if the behavior is intentional for the quickstart, or file a follow-up issue.

Suggestions (2)

  1. PR title convention: The title Style: Pass 'make lint' uses a capitalized Style: prefix. The kagenti org convention is lowercase conventional commit prefixes. Consider style: pass make lint to be consistent.

  2. Follow-up: Add lint to CI: As the PR description itself notes, make lint is not part of CI, which allowed this drift. A follow-up to add make lint as a CI step (or at minimum as part of pre-commit) would prevent recurrence. The Pre-commit Checks job already passes, so this may already be partially covered -- worth confirming that ruff and ruff format are in the pre-commit config.

Positive observations

  • The single-commit approach is clean and easy to review.
  • The f-string quoting fixes (client_payload["clientId"] -> client_payload['clientId']) are genuine correctness improvements for Python <3.12 compatibility.
  • Removing the unused KeycloakGetError import is a good cleanup.
  • The reformatting in keycloak_sync.py (argparse section) significantly improves readability despite the higher line count -- flattening excessively wrapped argument definitions.

CI Status

Check Status
CodeQL FAIL (pre-existing high: clear-text secret logging in quickstart)
DCO Pass
Go CI (all 3 jobs) Pass
Python Tests Pass
Pre-commit Checks Pass
Dependency Review Pass
Trivy Filesystem Scan Pass
Python Security (Bandit) Pass
Dockerfile Lint (Hadolint) Pass
YAML Lint Pass
Shell Script Lint Pass
Verify Action Pinning Pass
Spellcheck Skipping

Verdict: COMMENT

The code changes themselves are clean, mechanical formatting fixes with no behavioral changes. The only CI failure (CodeQL) is a pre-existing issue not introduced by this PR. I would approve once the CodeQL finding is acknowledged -- either suppressed inline for the quickstart script, or tracked as a separate issue so it does not block this formatting cleanup.

Copy link
Copy Markdown
Collaborator

@cwiklik cwiklik left a comment

Choose a reason for hiding this comment

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

Pure style/formatting PR that brings the codebase in line with make lint (ruff). No behavioral changes — only formatting, trailing commas, quote normalization, f-string-without-vars fixes, one unused import removal (KeycloakGetError), and missing newlines at EOF.

All CI checks pass. LGTM.

@esnible esnible merged commit 0043221 into kagenti:main Apr 29, 2026
17 of 18 checks passed
@esnible esnible deleted the pass-make-lint branch April 29, 2026 13:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

🐛 "make lint" fails

3 participants