HYPERFLEET-1290 - fix: restore backward compat for precondition api_call

[rh-amarin]

Summary

• HYPERFLEET-1290 introduced a hard validation error when api_call is used directly on a precondition, breaking existing adapter configs without a migration path
• The precondition executor already handles api_call at runtime (code was not removed), so only the validator was rejecting it
• This PR downgrades the error to a deprecation warning logged at startup, keeping existing configs working while steering authors toward the new params[].source.api_call pattern

Changes

• internal/configloader/validator.go: Added warnings []string field and Warnings() method to TaskConfigValidator; changed validatePreconditionAPICallForbidden to append to warnings instead of errors
• internal/configloader/loader.go: Log collected warnings via the instance logger after semantic validation
• internal/configloader/validator_test.go: Updated test to assert a deprecation warning is emitted (not an error) when a precondition uses api_call

Test plan

• make build — binary compiles cleanly
• make test — all unit tests pass
• Adapter config with old-style precondition api_call loads and logs a DEPRECATED warning at startup instead of refusing to start
• Adapter config using new-style params[].source.api_call continues to work with no warnings

🤖 Generated with Claude Code

[openshift-ci]

[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 mbrudnoy 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:

• OWNERS

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

[coderabbitai]

📝 Walkthrough

Walkthrough

TaskConfigValidator now collects deprecation messages in a warnings field, exposed via a new Warnings() method. Deprecated api_call usage in preconditions no longer produces a validation error but instead appends a deprecation warning with migration guidance. The config loader logs these warnings after successful semantic validation. Tests are updated to assert successful validation with the expected warning content instead of an error.

Estimated code review effort: 2 (Simple) | ~10 minutes

🚥 Pre-merge checks | ✅ 10 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (10 passed)

Check name	Status	Explanation
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.
Sec-02: Secrets In Log Output	✅ Passed	No non-test log call in the changed files interpolates or fields token/password/credential/secret; the new Warn path only logs api_call deprecation text.
No Hardcoded Secrets	✅ Passed	No hardcoded credentials, keys, credentialed URLs, or long base64 literals were found in the three changed files (CWE-798/CWE-321).
No Weak Cryptography	✅ Passed	No banned crypto primitives, ECB, custom crypto, or secret comparisons were added; only config validation/logging changed. No CWE-327/CWE-328/CWE-916.
No Injection Vectors	✅ Passed	No new CWE-78/79/89/502 sink was introduced; the diff only adds warning collection/logging. No exec.Command, template.HTML, or query concatenation in changed code.
No Privileged Containers	✅ Passed	HEAD only changes Go configloader files; no Kubernetes/OpenShift manifests, Helm templates, or Dockerfiles were modified, so no privileged-container flags to review.
No Pii Or Sensitive Data In Logs	✅ Passed	No CWE-532 exposure: the new warning logs static deprecation text plus a resourcename-constrained precondition name; no PII, bodies, headers, or secrets.
Title check	✅ Passed	The title is concise and accurately describes the backward-compatibility fix for precondition api_call.
Description check	✅ Passed	The description matches the code changes and test updates for downgrading api_call validation to a warning.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

✨ Simplify code

• Create PR with simplified code

---

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

[hyperfleet-ci-bot]

Risk Score: 0 — risk/low

Signal	Detail	Points
PR size	32 lines	+0
Sensitive paths	none	+0
Test coverage	Tests cover changed packages	+0

_{Computed by hyperfleet-risk-scorer}

[coderabbitai]

🧹 Nitpick comments (1)

internal/configloader/loader.go (1)

179-187: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

No test coverage for the new warning-logging path.

taskValidator.Warnings() is now iterated and logged, but no test in the provided files exercises LoadConfig with a deprecated api_call precondition to confirm the warning is actually surfaced through the logger. The logic itself is correct.

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

In `@internal/configloader/loader.go` around lines 179 - 187, Add test coverage
for the warning-logging path in LoadConfig by exercising
taskValidator.Warnings() with a deprecated api_call precondition and asserting
the logger receives the warning. Update or add a test around LoadConfig in
configloader/loader.go that triggers ValidateSemantic() successfully, then
verifies o.logger.Warn is called with the expected deprecation message.

Source: Path instructions

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

Nitpick comments:
In `@internal/configloader/loader.go`:
- Around line 179-187: Add test coverage for the warning-logging path in
LoadConfig by exercising taskValidator.Warnings() with a deprecated api_call
precondition and asserting the logger receives the warning. Update or add a test
around LoadConfig in configloader/loader.go that triggers ValidateSemantic()
successfully, then verifies o.logger.Warn is called with the expected
deprecation message.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Central YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: adfaed86-5be1-47c3-a6b4-d91bb395baa8

📥 Commits

Reviewing files that changed from the base of the PR and between 95da817 and a7ab082.

📒 Files selected for processing (3)

• internal/configloader/loader.go
• internal/configloader/validator.go
• internal/configloader/validator_test.go

🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

• openshift-hyperfleet/architecture (manual)
• openshift-hyperfleet/hyperfleet-api (manual)
• openshift-hyperfleet/hyperfleet-sentinel (manual)
• openshift-hyperfleet/hyperfleet-adapter (manual)
• openshift-hyperfleet/hyperfleet-broker (manual)

[openshift-ci]

@rh-amarin: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/lint	a7ab082	link	true	/test lint

Full PR test history. Your PR dashboard.

Details

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. I understand the commands that are listed here.