[BUG] BuildRun conversion ignores deprecated .status.failedAt from v1alpha1

[Ryntak94]

Is there an existing issue for this?

• I have searched the existing issues

Kubernetes Version

n/a (API conversion logic, reproducible as a unit test)

Shipwright Version

v0.20.13 / main @ 0d8ae09

Current Behavior

BuildRun conversion handles the deprecated .status.failedAt field in only one direction.

v1beta1 → v1alpha1 (ConvertTo) writes it. buildrun_conversion.go#L182-L189 deliberately mirrors FailureDetails.Location into the deprecated field, with an explicit opt-out of the staticcheck deprecation warning:

//nolint:staticcheck // SA1019 we want to give users some time to adopt to failureDetails
alphaBuildRun.Status.FailedAt = alphaBuildRun.Status.FailureDetails.Location

v1alpha1 → v1beta1 (ConvertFrom) never reads it. buildrun_conversion.go#L256-L262 only looks at FailureDetails:

if alphaBuildRun.Status.FailureDetails != nil {
	src.Status.FailureDetails = &FailureDetails{
		Reason:   alphaBuildRun.Status.FailureDetails.Reason,
		Message:  alphaBuildRun.Status.FailureDetails.Message,
		Location: (*Location)(alphaBuildRun.Status.FailureDetails.Location),
	}
}

So a v1alpha1 BuildRun that populated only .status.failedAt — which is exactly what a client written against the older API would do, and what this codebase itself emitted before failureDetails existed — converts to a v1beta1 object with no failure location at all. .status.failureDetails comes out nil.

Expected Behavior

ConvertFrom should fall back to .status.failedAt when .status.failureDetails is absent, populating .status.failureDetails.location from it. That would make the deprecated field round-trip, consistent with ConvertTo already going out of its way to keep emitting it.

Steps To Reproduce

Converting a v1alpha1 BuildRun that sets only status.failedAt drops the location:

func TestFailedAtDropped(t *testing.T) {
	alpha := &buildapialpha.BuildRun{
		Status: buildapialpha.BuildRunStatus{
			FailedAt: &buildapialpha.FailedAt{Pod: "buildrun-pod", Container: "step-build"},
		},
	}
	alpha.APIVersion = "shipwright.io/v1alpha1"
	alpha.Kind = "BuildRun"

	raw, _ := runtime.DefaultUnstructuredConverter.ToUnstructured(alpha)
	u := &unstructured.Unstructured{Object: raw}

	beta := &BuildRun{}
	if err := beta.ConvertFrom(context.TODO(), u); err != nil {
		t.Fatal(err)
	}

	if beta.Status.FailureDetails == nil {
		t.Error("status.failureDetails is nil; the failure location was dropped")
	}
}

Output:

INPUT  v1alpha1: status.failedAt = &{Pod:buildrun-pod Container:step-build}
--- FAIL: TestFailedAtDropped
    status.failureDetails is nil; the failure location was dropped

As a control, the opposite direction confirms the asymmetry is real — ConvertTo on a v1beta1 object with FailureDetails.Location set produces:

ConvertTo -> v1alpha1 status.failedAt found=true value=map[container:step-build pod:buildrun-pod]

Anything else?

I recognise failedAt is deprecated, so it is possible that ignoring it on the way in is intentional. Two things suggest otherwise, which is why I am raising it:

1. ConvertTo does not merely tolerate the field, it actively populates it and suppresses the deprecation lint to do so, with a comment stating the intent is to give users time to migrate. Accepting the field from v1alpha1 producers seems like the matching half of that grace period.
2. The failure is silent. There is no warning and no error — the failure location simply disappears, which makes a failed BuildRun harder to debug precisely when the user needs that information.

If the current behaviour is intended, it may still be worth documenting that .status.failedAt is write-only across conversion.

I am happy to open a PR adding the fallback plus round-trip test coverage, if that would be welcome.

Found while auditing buildrun_conversion.go for the same class of issue as #2303.

[Ryntak94]

Cross-linking #1686 (Stop Serving v1alpha1 API), which I should have referenced when I filed this.

That issue notes that "other potential bugs related to v1alpha1 -> v1beta1 conversion can be closed out as obsolete" once v1alpha1 is no longer served, and this report sits squarely in that class. I filed it anyway because v1alpha1 is still served today and #1686 is currently blocked on shipwright-io/cli#294, so the grace period ConvertTo is explicitly maintaining for .status.failedAt is still live. But I would rather put that context in front of you than have it come up in triage.

If the deprecation is close enough that this is not worth carrying, please close it as obsolete — no objection from me. The same applies to #2303 and to the PRs opened against both.