fix(vm): align VM error schema and WriteProblem with RFC 9457

[gciavarrini]

Summary

OpenAPI request validation returned text/plain via http.Error.

POST /vms 400s now use application/problem+json with the same type/title/status/detail body as handler errors.
The Error schema comment now says problem details (RFC 9457), not RFC 7807.

Fixes

FLPATH-4751
(Replaces dcm-project/kubevirt-service-provider#36)

Summary by Sourcery

Return OpenAPI validation errors using the same structured problem details format as other API errors.

Bug Fixes:

• Return OpenAPI request validation failures as RFC 9457 problem details with an application/problem+json content type instead of plain text.

Enhancements:

• Align the Error schema and generated API documentation with RFC 9457 problem details.
• Centralize problem detail response writing for consistent validation and handler error responses.

Tests:

• Add coverage for problem details responses across empty, malformed, incorrectly typed, and schema-invalid VM requests.

[qodo-code-review]

PR Summary by Qodo

Align VM error contract with RFC 9457 problem details

🐞 Bug fix 🧪 Tests 📝 Documentation 🕐 10-20 Minutes

AI Description

• Adds an RFC 9457 writer for consistent VM problem responses.
• Sets application/problem+json with type, title, status, and detail fields.
• Refreshes generated OpenAPI artifacts and tests the response contract.

Diagram

graph TD
  A["OpenAPI source"] --> B["Generated contract"] --> C["Embedded spec"]
  B --> D["Error models"]
  E["Validation hook"] --> F["WriteProblem"] --> G["Problem response"]

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Reuse the shared HTTP problem writer

• ➕ Avoids maintaining another JSON response writer
• ➕ Keeps error logging and response behavior consistent across APIs
• ➖ The existing helper uses the main API Error model rather than the VM-generated type
• ➖ Its error-type and instance conventions do not directly match the VM about:blank response

Recommendation: Keeping a VM-local writer is reasonable because it constructs the generated VM Error type and preserves the existing about:blank contract. However, the shown diff has no production caller for WriteProblem; it must be connected to the VM/OpenAPI validation error handler for the stated text/plain bug to be fixed. A shared schema-neutral RFC 9457 writer would be preferable later if both APIs converge on identical semantics.

Files changed (7) +100 / -62

Bug fix (1) +20 / -1

errors.goAdd an RFC 9457 problem response writer +20/-1

Add an RFC 9457 problem response writer

• Adds WriteProblem to serialize the existing VM Error model before committing an application/problem+json response. Encoding and response-write failures are logged, with encoding failures falling back to HTTP 500.

internal/openshift/kubevirtvm/kubevirt/errors.go

Tests (1) +19 / -0

errors_test.goVerify the VM problem response contract +19/-0

Verify the VM problem response contract

• Tests that WriteProblem returns HTTP 400, application/problem+json, and the expected type, title, status, and detail fields.

internal/openshift/kubevirtvm/kubevirt/errors_test.go

Documentation (5) +61 / -61

openapi.source.yamlDescribe VM errors as RFC 9457 problem details +1/-1

Describe VM errors as RFC 9457 problem details

• Updates the authoritative Error schema description from RFC 7807 terminology to RFC 9457 problem details.

api/vm/v1alpha1/openapi.source.yaml

openapi.yamlRegenerate the published VM OpenAPI contract +1/-1

Regenerate the published VM OpenAPI contract

• Propagates the RFC 9457 Error schema description into the generated OpenAPI document.

api/vm/v1alpha1/openapi.yaml

spec.gen.goRefresh the embedded VM OpenAPI specification +57/-57

Refresh the embedded VM OpenAPI specification

• Regenerates the compressed OpenAPI payload embedded in the Go package after the schema description change.

api/vm/v1alpha1/spec.gen.go

types.gen.goUpdate the generated VM Error documentation +1/-1

Update the generated VM Error documentation

• Changes the generated public Error type comment to identify the schema as RFC 9457 problem details.

api/vm/v1alpha1/types.gen.go

server.gen.goRefresh generated server Error documentation +1/-1

Refresh generated server Error documentation

• Updates the generated server-side Error model comment to reference RFC 9457 problem details.

internal/openshift/kubevirtvm/oapi/server/server.gen.go

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (2) 📜 Skill insights (0)

1. Problem failures use the wrong logger 📘 Rule violation ⌂ Architecture

Description

WriteProblem calls slog.Default() directly instead of receiving the application's configured
logger as a dependency. When writing a response fails, its diagnostic bypasses the injected logging
configuration and any handlers or attributes attached to it.

Code

internal/openshift/kubevirtvm/kubevirt/errors.go[R69-70]

+	if _, err := buf.WriteTo(w); err != nil {
+		slog.Default().Error("failed to write problem+json", "error", err)

Evidence

Compliance rule 2788520 prohibits dependencies accessed through package-level globals. The new
response writer invokes the package-global slog.Default() for its error handling.

Rule 2788520: Use constructor injection, no globals or init()
internal/openshift/kubevirtvm/kubevirt/errors.go[58-70]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`WriteProblem` accesses the global default logger instead of using an injected application logger.

## Issue Context
Both error paths must report through the configured logger so application handlers and attributes are preserved.

## Fix Focus Areas
- internal/openshift/kubevirtvm/kubevirt/errors.go[58-72]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Validation routing remains untested 📘 Rule violation ▣ Testability

Description

The new WriteProblem test calls the helper directly with an httptest.ResponseRecorder, while no
production request-validation path invokes the function. OpenAPI validation and generated decoding
failures still flow through httperror.WriteInvalidArgument, and VM operations use the embedded
service-provider route rather than the generated /vms server, so the test can pass while real
invalid requests continue through the existing response mechanism.

Code

internal/openshift/kubevirtvm/kubevirt/errors_test.go[R58-59]

+			rec := httptest.NewRecorder()
+			kubevirt.WriteProblem(rec, http.StatusBadRequest, "Validation Error", "value is required but missing")

Evidence

The test citation shows that WriteProblem is exercised only as an isolated formatter, without
submitting an invalid request through the OpenAPI validation middleware. The production wiring
citations show that OpenAPI validation and generated decoding errors are handled by
httperror.WriteInvalidArgument, while VM operations are exposed through the embedded
service-provider handler instead of the generated /vms HTTP server; together, these references
establish that the test does not verify a production path and that WriteProblem has no production
caller.

Rule 2788542: Prefer integration tests over unit tests
internal/openshift/kubevirtvm/kubevirt/errors_test.go[56-70]
internal/apiserver/server.go[60-75]
cmd/environment-agent/main.go[280-291]
internal/embedded/vm/handler.go[31-50]
internal/openshift/kubevirtvm/oapi/server/server.gen.go[1382-1390]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description

Replace or supplement the direct `WriteProblem` unit test with integration coverage that submits invalid VM requests through the real OpenAPI validation middleware. Wire the intended production validation path to the structured problem-details writer, or remove `WriteProblem` if the existing shared `httperror` handling is authoritative.

## Issue Context

The current test invokes `WriteProblem` directly with an `httptest.ResponseRecorder`, so it verifies only formatting and can pass without changing responses to real invalid requests. The agent API server currently routes OpenAPI validation and generated request-decoding failures through `httperror.WriteInvalidArgument`, and VM operations are exposed through the embedded service-provider routing layer rather than the generated KubeVirt `/vms` handler. Integration coverage should assert the resulting status, `application/problem+json` content type, and RFC 9457 body through the actual production wiring.

## Fix Focus Areas

- internal/openshift/kubevirtvm/kubevirt/errors_test.go[56-70]
- internal/openshift/kubevirtvm/kubevirt/errors.go[58-72]
- internal/apiserver/server.go[60-75]
- cmd/environment-agent/main.go[280-290]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Context sources

⚠️ Tickets: not configured — ticket URL found in PR but could not be fetched — check ticket provider credentials

✅ Compliance rules (platform): 17 rules

Review mode: ⚖️ Balanced: This changes runtime HTTP error handling and a public API response contract, so it warrants a careful single-pass review despite the localized implementation.

Tip of the day

💡 Did you know, you can commit Qodo's fix in one click with committable suggestions (GitHub & GitLab)

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

[qodo-code-review]

1. Problem failures use the wrong logger 📘 Rule violation ⌂ Architecture

WriteProblem calls slog.Default() directly instead of receiving the application's configured
logger as a dependency. When writing a response fails, its diagnostic bypasses the injected logging
configuration and any handlers or attributes attached to it.

Agent Prompt

## Issue description
`WriteProblem` accesses the global default logger instead of using an injected application logger.

## Issue Context
Both error paths must report through the configured logger so application handlers and attributes are preserved.

## Fix Focus Areas
- internal/openshift/kubevirtvm/kubevirt/errors.go[58-72]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

[qodo-code-review]

2. Validation routing remains untested 📘 Rule violation ▣ Testability

The new WriteProblem test calls the helper directly with an httptest.ResponseRecorder, while no
production request-validation path invokes the function. OpenAPI validation and generated decoding
failures still flow through httperror.WriteInvalidArgument, and VM operations use the embedded
service-provider route rather than the generated /vms server, so the test can pass while real
invalid requests continue through the existing response mechanism.

Agent Prompt

## Issue description

Replace or supplement the direct `WriteProblem` unit test with integration coverage that submits invalid VM requests through the real OpenAPI validation middleware. Wire the intended production validation path to the structured problem-details writer, or remove `WriteProblem` if the existing shared `httperror` handling is authoritative.

## Issue Context

The current test invokes `WriteProblem` directly with an `httptest.ResponseRecorder`, so it verifies only formatting and can pass without changing responses to real invalid requests. The agent API server currently routes OpenAPI validation and generated request-decoding failures through `httperror.WriteInvalidArgument`, and VM operations are exposed through the embedded service-provider routing layer rather than the generated KubeVirt `/vms` handler. Integration coverage should assert the resulting status, `application/problem+json` content type, and RFC 9457 body through the actual production wiring.

## Fix Focus Areas

- internal/openshift/kubevirtvm/kubevirt/errors_test.go[56-70]
- internal/openshift/kubevirtvm/kubevirt/errors.go[58-72]
- internal/apiserver/server.go[60-75]
- cmd/environment-agent/main.go[280-290]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

[gabriel-farache]

I think the "high" level comment from Qodo makes sense, otherwise LGTM