Skip to content

Fix resolutionrequest conversion #6509

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 11, 2023
Merged

Fix resolutionrequest conversion #6509

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions cmd/webhook/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,9 @@ func newConversionController(ctx context.Context, cmw configmap.Watcher) *contro
"/resource-conversion",

// Specify the types of custom resource definitions that should be converted
// "HubVersion" is the stored version, and "Zygotes" are the supported versions
// "HubVersion" specifies which version of the CustomResource supports
// conversions to and from all types.
// "Zygotes" are the supported versions.
map[schema.GroupKind]conversion.GroupKindConversion{
v1.Kind("Task"): {
DefinitionName: pipeline.TaskResource.String(),
Expand Down Expand Up @@ -202,7 +204,7 @@ func newConversionController(ctx context.Context, cmw configmap.Watcher) *contro
},
resolutionv1beta1.Kind("ResolutionRequest"): {
DefinitionName: resolution.ResolutionRequestResource.String(),
HubVersion: resolutionv1beta1GroupVersion,
HubVersion: resolutionv1alpha1GroupVersion,
Zygotes: map[string]conversion.ConvertibleObject{
resolutionv1alpha1GroupVersion: &resolutionv1alpha1.ResolutionRequest{},
resolutionv1beta1GroupVersion: &resolutionv1beta1.ResolutionRequest{},
Expand Down
45 changes: 45 additions & 0 deletions pkg/apis/resolution/v1alpha1/resolution_request_conversion.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ func (rr *ResolutionRequest) ConvertTo(ctx context.Context, sink apis.Convertibl
switch sink := sink.(type) {
case *v1beta1.ResolutionRequest:
sink.ObjectMeta = rr.ObjectMeta
rr.Status.convertTo(ctx, &sink.Status)
return rr.Spec.ConvertTo(ctx, &sink.Spec)
default:
return fmt.Errorf("unknown version, got: %T", sink)
Expand All @@ -58,6 +59,22 @@ func (rrs *ResolutionRequestSpec) ConvertTo(ctx context.Context, sink *v1beta1.R
return nil
}

// convertTo converts a v1alpha1.ResolutionRequestStatus to a v1beta1.ResolutionRequestStatus
func (rrs *ResolutionRequestStatus) convertTo(ctx context.Context, sink *v1beta1.ResolutionRequestStatus) {
sink.Data = rrs.Data
if rrs.RefSource != nil {
refSource := pipelinev1beta1.RefSource{}
refSource.URI = rrs.RefSource.URI
refSource.EntryPoint = rrs.RefSource.EntryPoint
digest := make(map[string]string)
for k, v := range rrs.RefSource.Digest {
digest[k] = v
}
refSource.Digest = digest
sink.RefSource = &refSource
}
}

// ConvertFrom implements apis.Convertible
func (rr *ResolutionRequest) ConvertFrom(ctx context.Context, from apis.Convertible) error {
if apis.IsInDelete(ctx) {
Expand All @@ -66,6 +83,7 @@ func (rr *ResolutionRequest) ConvertFrom(ctx context.Context, from apis.Converti
switch from := from.(type) {
case *v1beta1.ResolutionRequest:
rr.ObjectMeta = from.ObjectMeta
rr.Status.convertFrom(ctx, &from.Status)
return rr.Spec.ConvertFrom(ctx, &from.Spec)
default:
return fmt.Errorf("unknown version, got: %T", from)
Expand Down Expand Up @@ -93,3 +111,30 @@ func (rrs *ResolutionRequestSpec) ConvertFrom(ctx context.Context, from *v1beta1

return nil
}

// convertTo converts a v1alpha1.ResolutionRequestStatus to a v1beta1.ResolutionRequestStatus
func (rrs *ResolutionRequestStatus) convertFrom(ctx context.Context, from *v1beta1.ResolutionRequestStatus) {
rrs.Data = from.Data

if from.RefSource != nil {
refSource := pipelinev1beta1.RefSource{}
refSource.URI = from.RefSource.URI
refSource.EntryPoint = from.RefSource.EntryPoint
digest := make(map[string]string)
for k, v := range from.RefSource.Digest {
digest[k] = v
}
refSource.Digest = digest
rrs.RefSource = &refSource
} else if from.Source != nil {
refSource := pipelinev1beta1.RefSource{}
refSource.URI = from.Source.URI
refSource.EntryPoint = from.Source.EntryPoint
digest := make(map[string]string)
for k, v := range from.Source.Digest {
digest[k] = v
}
refSource.Digest = digest
rrs.RefSource = &refSource
}
}
263 changes: 117 additions & 146 deletions pkg/apis/resolution/v1alpha1/resolution_request_conversion_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ package v1alpha1

import (
"context"
"errors"
"testing"

"github.com/google/go-cmp/cmp"
Expand All @@ -42,182 +41,154 @@ func TestResolutionRequestConversionBadType(t *testing.T) {
}
}

func TestResolutionRequestConvertTo(t *testing.T) {
versions := []apis.Convertible{&v1beta1.ResolutionRequest{}}

func TestResolutionRequestConvertRoundTrip(t *testing.T) {
testCases := []struct {
name string
in *ResolutionRequest
out apis.Convertible
}{
{
name: "no params",
in: &ResolutionRequest{
ObjectMeta: metav1.ObjectMeta{
Name: "foo",
Namespace: "bar",
},
Spec: ResolutionRequestSpec{
Parameters: nil,
},
}{{
name: "no params",
in: &ResolutionRequest{
ObjectMeta: metav1.ObjectMeta{
Name: "foo",
Namespace: "bar",
},
out: &v1beta1.ResolutionRequest{
ObjectMeta: metav1.ObjectMeta{
Name: "foo",
Namespace: "bar",
},
Spec: v1beta1.ResolutionRequestSpec{
Params: nil,
Spec: ResolutionRequestSpec{
Parameters: nil,
},
},
}, {
name: "with params",
in: &ResolutionRequest{
ObjectMeta: metav1.ObjectMeta{
Name: "foo",
Namespace: "bar",
},
Spec: ResolutionRequestSpec{
Parameters: map[string]string{
"some-param": "some-value",
},
},
}, {
name: "with params",
in: &ResolutionRequest{
ObjectMeta: metav1.ObjectMeta{
Name: "foo",
Namespace: "bar",
},
}, {
name: "with status refsource",
in: &ResolutionRequest{
ObjectMeta: metav1.ObjectMeta{
Name: "foo",
Namespace: "bar",
},
Spec: ResolutionRequestSpec{
Parameters: map[string]string{
"some-param": "some-value",
},
Spec: ResolutionRequestSpec{
Parameters: map[string]string{
"some-param": "some-value",
},
Status: ResolutionRequestStatus{
ResolutionRequestStatusFields: ResolutionRequestStatusFields{
Data: "foobar",
RefSource: &pipelinev1beta1.RefSource{
URI: "abcd",
Digest: map[string]string{"123": "456"},
EntryPoint: "baz",
},
},
},
out: &v1beta1.ResolutionRequest{
ObjectMeta: metav1.ObjectMeta{
Name: "foo",
Namespace: "bar",
},
}, {
name: "with status, no refsource",
in: &ResolutionRequest{
ObjectMeta: metav1.ObjectMeta{
Name: "foo",
Namespace: "bar",
},
Spec: ResolutionRequestSpec{
Parameters: map[string]string{
"some-param": "some-value",
},
Spec: v1beta1.ResolutionRequestSpec{
Params: []pipelinev1beta1.Param{{
Name: "some-param",
Value: *pipelinev1beta1.NewStructuredValues("some-value"),
}},
},
Status: ResolutionRequestStatus{
ResolutionRequestStatusFields: ResolutionRequestStatusFields{
Data: "foobar",
},
},
},
}
}}

for _, tc := range testCases {
for _, version := range versions {
t.Run(tc.name, func(t *testing.T) {
got := version
if err := tc.in.ConvertTo(context.Background(), got); err != nil {
t.Fatalf("ConvertTo() = %v", err)
}
t.Logf("ConvertTo() = %#v", got)
if d := cmp.Diff(tc.out, got); d != "" {
t.Errorf("converted ResolutionRequest did not match expected: %s", diff.PrintWantGot(d))
}
})
}
t.Run(tc.name, func(t *testing.T) {
got := &v1beta1.ResolutionRequest{}
if err := tc.in.ConvertTo(context.Background(), got); err != nil {
t.Fatalf("ConvertTo() = %v", err)
}

t.Logf("ConvertTo() = %#v", got)

roundTrip := &ResolutionRequest{}
if err := roundTrip.ConvertFrom(context.Background(), got); err != nil {
t.Errorf("ConvertFrom() = %v", err)
}

if d := cmp.Diff(tc.in, roundTrip); d != "" {
t.Errorf("converted ResolutionRequest did not match expected: %s", diff.PrintWantGot(d))
}
})
}
}

func TestResolutionRequestConvertFrom(t *testing.T) {
versions := []apis.Convertible{&ResolutionRequest{}}

func TestResolutionRequestConvertFromDeprecated(t *testing.T) {
testCases := []struct {
name string
in apis.Convertible
out *ResolutionRequest
expectedErr error
}{
{
name: "no params",
in: &v1beta1.ResolutionRequest{
ObjectMeta: metav1.ObjectMeta{
Name: "foo",
Namespace: "bar",
},
Spec: v1beta1.ResolutionRequestSpec{
Params: nil,
},
},
out: &ResolutionRequest{
ObjectMeta: metav1.ObjectMeta{
Name: "foo",
Namespace: "bar",
},
Spec: ResolutionRequestSpec{
Parameters: nil,
},
name string
in *v1beta1.ResolutionRequest
want apis.Convertible
}{{
name: "with status.source",
in: &v1beta1.ResolutionRequest{
ObjectMeta: metav1.ObjectMeta{
Name: "foo",
Namespace: "bar",
},
}, {
name: "with only string params",
in: &v1beta1.ResolutionRequest{
ObjectMeta: metav1.ObjectMeta{
Name: "foo",
Namespace: "bar",
},
Spec: v1beta1.ResolutionRequestSpec{
Params: []pipelinev1beta1.Param{{
Name: "some-param",
Value: *pipelinev1beta1.NewStructuredValues("some-value"),
}},
},
Spec: v1beta1.ResolutionRequestSpec{
Params: nil,
},
out: &ResolutionRequest{
ObjectMeta: metav1.ObjectMeta{
Name: "foo",
Namespace: "bar",
},
Spec: ResolutionRequestSpec{
Parameters: map[string]string{
"some-param": "some-value",
Status: v1beta1.ResolutionRequestStatus{
ResolutionRequestStatusFields: v1beta1.ResolutionRequestStatusFields{
Source: &pipelinev1beta1.ConfigSource{
URI: "abcd",
Digest: map[string]string{"123": "456"},
EntryPoint: "baz",
},
},
},
}, {
name: "with non-string params",
in: &v1beta1.ResolutionRequest{
ObjectMeta: metav1.ObjectMeta{
Name: "foo",
Namespace: "bar",
},
Spec: v1beta1.ResolutionRequestSpec{
Params: []pipelinev1beta1.Param{
{
Name: "array-val",
Value: *pipelinev1beta1.NewStructuredValues("one", "two"),
}, {
Name: "string-val",
Value: *pipelinev1beta1.NewStructuredValues("a-string"),
}, {
Name: "object-val",
Value: *pipelinev1beta1.NewObject(map[string]string{
"key-one": "value-one",
"key-two": "value-two",
}),
},
},
want: &ResolutionRequest{
ObjectMeta: metav1.ObjectMeta{
Name: "foo",
Namespace: "bar",
},
Spec: ResolutionRequestSpec{
Parameters: nil,
},
Status: ResolutionRequestStatus{
ResolutionRequestStatusFields: ResolutionRequestStatusFields{
RefSource: &pipelinev1beta1.RefSource{
URI: "abcd",
Digest: map[string]string{"123": "456"},
EntryPoint: "baz",
},
},
},
out: nil,
expectedErr: errors.New("cannot convert v1beta1 to v1alpha, non-string type parameter(s) found: array-val, object-val"),
},
}
}}

for _, tc := range testCases {
for _, version := range versions {
t.Run(tc.name, func(t *testing.T) {
got := version
err := got.ConvertFrom(context.Background(), tc.in)
if tc.expectedErr != nil {
if err == nil {
t.Fatalf("expected error '%s', but did not get an error", tc.expectedErr.Error())
} else if d := cmp.Diff(tc.expectedErr.Error(), err.Error()); d != "" {
t.Fatalf("error did not meet expected: %s", diff.PrintWantGot(d))
}
return
} else if err != nil {
t.Fatalf("ConvertFrom() = %v", err)
}
t.Logf("ConvertFrom() = %#v", got)
if d := cmp.Diff(tc.out, got); d != "" {
t.Errorf("converted ResolutionRequest did not match expected: %s", diff.PrintWantGot(d))
}
})
}
t.Run(tc.name, func(t *testing.T) {
got := &ResolutionRequest{}
if err := got.ConvertFrom(context.Background(), tc.in); err != nil {
t.Errorf("ConvertFrom() = %v", err)
}

if d := cmp.Diff(tc.want, got); d != "" {
t.Errorf("converted ResolutionRequest did not match expected: %s", diff.PrintWantGot(d))
}
})
}
}