Skip to content

Add SSS enabled/disabled switch in EasyCLA#5104

Merged
lukaszgryglicki merged 3 commits into
devfrom
unicron-sss-enabled-flag
Jul 1, 2026
Merged

Add SSS enabled/disabled switch in EasyCLA#5104
lukaszgryglicki merged 3 commits into
devfrom
unicron-sss-enabled-flag

Conversation

@lukaszgryglicki

Copy link
Copy Markdown
Member

Signed-off-by: Lukasz Gryglicki lgryglicki@cncf.io

Assisted by OpenAI

Assisted by GitHub Copilot

Assisted by Claude

Signed-off-by: Lukasz Gryglicki <lgryglicki@cncf.io>

Assisted by [OpenAI](https://platform.openai.com/)

Assisted by [GitHub Copilot](https://github.com/features/copilot)

Assisted by [Claude](https://claude.ai)
@lukaszgryglicki lukaszgryglicki self-assigned this Jul 1, 2026
Copilot AI review requested due to automatic review settings July 1, 2026 04:39
@coderabbitai

coderabbitai Bot commented Jul 1, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: d4dafdc5-120f-42b0-9c88-b9fed9670555

📥 Commits

Reviewing files that changed from the base of the PR and between 4af43f4 and 4494fb6.

📒 Files selected for processing (3)
  • cla-backend-go/config/config.go
  • cla-backend-go/config/ssm.go
  • cla-backend-legacy/internal/api/handlers.go
🚧 Files skipped from review as they are similar to previous changes (3)
  • cla-backend-go/config/ssm.go
  • cla-backend-go/config/config.go
  • cla-backend-legacy/internal/api/handlers.go

Walkthrough

Adds an Enabled master-switch field to the SSS configuration, loads it from SSM with a defaulting boolean helper, threads sssEnabled through service wiring, and gates SSS startup and screening logic on that flag in both backend paths.

Changes

SSS Enabled Kill Switch

Layer / File(s) Summary
SSS.Enabled config field and SSM loader
cla-backend-go/config/config.go, cla-backend-go/config/ssm.go
Adds SSS.Enabled and a defaulting SSM boolean reader used by loadOptionalSSSConfig.
sign.NewService sssEnabled parameter and screening bypass
cla-backend-go/v2/sign/service.go
NewService gains sssEnabled, stores it on the service, and checkCompanyCompliance returns early when it is false.
server.go and s3_upload wiring of sssEnabled
cla-backend-go/cmd/server.go, cla-backend-go/cmd/s3_upload/main.go
server() reads configFile.SSS.Enabled, adjusts SSS client startup handling, and passes sssEnabled into sign.NewService; s3_upload/main.go updates its constructor call to match.
Legacy Handlers sssEnabled gating
cla-backend-legacy/internal/api/handlers.go
Adds sssEnabled to Handlers, loads it from SSM, gates startup checks, and skips checkCompanyCompliance() when disabled.
SSS service test updates
cla-backend-go/v2/sign/service_sss_test.go
Updates existing SSS tests to set sssEnabled and adds coverage for the disabled skip path.

Estimated code review effort: 3 (Moderate) | ~25 minutes

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 77.78% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check ❓ Inconclusive The description contains only sign-off and attribution lines, so it doesn't convey the PR's changes. Replace it with a short summary of the SSS enable/disable changes and their impact.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: adding an SSS enabled/disabled switch.
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.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch unicron-sss-enabled-flag

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 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 `@cla-backend-go/cmd/server.go`:
- Around line 458-468: Restore the blank-config guard in server startup before
calling sss.NewClientFromPlatformCredentials so empty SSS settings are treated
as unconfigured rather than attempted initialization. In the startup flow around
sssEnabled/sssRequired and sssClient, check the SSS config fields first and skip
client creation when BaseURL or Audience are blank, leaving sssClient nil for
optional/disabled setups. Keep the existing fatal/warn handling only for actual
client निर्माण failures, not for missing configuration.

In `@cla-backend-legacy/internal/api/handlers.go`:
- Around line 284-290: The boolean parsing in getOptionalSSMBoolDefault
currently treats any non-"true" SSM value as false, which can silently disable
SSS on typos. Update this helper to explicitly accept only valid boolean strings
(for example true/false) and handle anything else by returning the safe default
or failing closed. Keep the change localized to getOptionalSSMBoolDefault and
its call path from getOptionalSSMString so malformed cla-sss-enabled-<stage>
values do not bypass sanctions screening.
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 38ba04a5-8183-4a95-ad11-90e4c9fb0fbe

📥 Commits

Reviewing files that changed from the base of the PR and between 02cdecf and 306b3c6.

📒 Files selected for processing (6)
  • cla-backend-go/cmd/s3_upload/main.go
  • cla-backend-go/cmd/server.go
  • cla-backend-go/config/config.go
  • cla-backend-go/config/ssm.go
  • cla-backend-go/v2/sign/service.go
  • cla-backend-legacy/internal/api/handlers.go

Comment thread cla-backend-go/cmd/server.go
Comment thread cla-backend-legacy/internal/api/handlers.go

Copilot AI left a comment

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.

Pull request overview

This PR adds a master enable/disable "kill switch" for the Sanctions Screening Service (SSS) in EasyCLA, controlled by a new cla-sss-enabled-{stage} SSM parameter that defaults to true so a not-yet-provisioned key never silently disables screening. When disabled, checkCompanyCompliance skips the live SSS call (after the manual/admin block short-circuit) and returns non-blocking. The change is mirrored across the Go backend (cla-backend-go) and the legacy backend (cla-backend-legacy).

Changes:

  • Add SSS.Enabled config field plus a getOptionalSSMBoolDefault helper (defaults to a caller-supplied value) in both backends.
  • Thread sssEnabled through the v2 sign service constructor and its call sites, and add an early skip in checkCompanyCompliance when disabled.
  • Gate the "required SSS" fatal startup checks on sssEnabled so the kill switch also relaxes startup enforcement.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
cla-backend-go/config/config.go Adds Enabled field to the SSS config struct with documentation of the kill-switch semantics.
cla-backend-go/config/ssm.go Loads cla-sss-enabled-{stage} (default true) via a new getOptionalSSMBoolDefault helper.
cla-backend-go/cmd/server.go Reads SSS.Enabled, gates fatal SSS checks on it, and passes it to sign.NewService.
cla-backend-go/cmd/s3_upload/main.go Updates the sign.NewService call to pass the new sssEnabled argument (false).
cla-backend-go/v2/sign/service.go Adds sssEnabled to the service, constructor, and an early skip in checkCompanyCompliance.
cla-backend-legacy/internal/api/handlers.go Mirrors the enabled flag, SSM default helper, startup gating, and compliance skip in legacy.

Comment thread cla-backend-go/v2/sign/service.go
Comment thread cla-backend-go/config/config.go Outdated
Comment thread cla-backend-go/config/ssm.go
Signed-off-by: Lukasz Gryglicki <lgryglicki@cncf.io>

Assisted by [OpenAI](https://platform.openai.com/)

Assisted by [GitHub Copilot](https://github.com/features/copilot)

Assisted by [Claude](https://claude.ai)

Copilot AI left a comment

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.

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Comment thread cla-backend-legacy/internal/api/handlers.go
Comment thread cla-backend-go/config/ssm.go
Signed-off-by: Lukasz Gryglicki <lgryglicki@cncf.io>

Assisted by [OpenAI](https://platform.openai.com/)

Assisted by [GitHub Copilot](https://github.com/features/copilot)

Assisted by [Claude](https://claude.ai)

Copilot AI left a comment

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.

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.

@lukaszgryglicki lukaszgryglicki merged commit 16491c4 into dev Jul 1, 2026
15 checks passed
@lukaszgryglicki lukaszgryglicki deleted the unicron-sss-enabled-flag branch July 1, 2026 12:22
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.

3 participants