diff --git a/apis/resources/v1alpha1/route_types.go b/apis/resources/v1alpha1/route_types.go index 28796300..63dddbda 100755 --- a/apis/resources/v1alpha1/route_types.go +++ b/apis/resources/v1alpha1/route_types.go @@ -104,6 +104,12 @@ type RouteStatus struct { AtProvider RouteObservation `json:"atProvider,omitempty"` } +// External-Name Configuration: +// - Follows Standard: yes +// - Format: Route GUID (UUID format) +// - How to find: +// - UI: Not available in the BTP Cockpit +// - CLI: Use CF CLI: `cf routes` and find the GUID in the output // +kubebuilder:object:root=true // +kubebuilder:storageversion @@ -163,3 +169,8 @@ func (r *Route) GetCloudFoundryName() string { func (r *Route) GetDomainRef() *DomainReference { return &r.Spec.ForProvider.DomainReference } + +// implement SpaceScoped interface +func (r *Route) GetSpaceRef() *SpaceReference { + return &r.Spec.ForProvider.SpaceReference +} diff --git a/internal/clients/resource_test.go b/internal/clients/resource_test.go new file mode 100644 index 00000000..15b31f18 --- /dev/null +++ b/internal/clients/resource_test.go @@ -0,0 +1,36 @@ +package clients + +import ( + "testing" + + "github.com/google/go-cmp/cmp" +) + +func TestIsValidGUID(t *testing.T) { + cases := map[string]struct { + guid string + valid bool + }{ + "valid GUID": { + guid: "33fd5b0b-4f3b-4b1b-8b3d-3b5f7b4b3b4b", + valid: true, + }, + "invalid GUID": { + guid: "not-a-valid-guid", + valid: false, + }, + "empty string": { + guid: "", + valid: false, + }, + } + + for n, tc := range cases { + t.Run(n, func(t *testing.T) { + result := IsValidGUID(tc.guid) + if diff := cmp.Diff(tc.valid, result); diff != "" { + t.Errorf("IsValidGUID(...): -want, +got:\n%s", diff) + } + }) + } +} diff --git a/internal/clients/route/route.go b/internal/clients/route/route.go index 8d973e5e..41e4b7ee 100644 --- a/internal/clients/route/route.go +++ b/internal/clients/route/route.go @@ -10,6 +10,7 @@ import ( "github.com/SAP/crossplane-provider-cloudfoundry/apis/resources/v1alpha1" "github.com/SAP/crossplane-provider-cloudfoundry/internal/clients" + "github.com/SAP/crossplane-provider-cloudfoundry/internal/clients/job" ) // Route is the interface that defines the methods that a Route client should implement. @@ -25,36 +26,41 @@ type Client struct { Route } -// NewClient creates a new cf client and return interfaces for Route and RouteFeatures -func NewClient(cf *client.Client) *Client { +// NewClient creates a new cf client and returns the Route client and Job client. +func NewClient(cf *client.Client) (*Client, job.Job) { return &Client{ Route: cf.Routes, - } + }, cf.Jobs } -// GetByIDOrSpec returns a Route by ID or by forProvider Spec. -func (c *Client) GetByIDOrSpec(ctx context.Context, guid string, forProvider v1alpha1.RouteParameters) (*v1alpha1.RouteObservation, error) { - var r *resource.Route - var err error - if clients.IsValidGUID(guid) { - r, err = c.Get(ctx, guid) - } else { - opts, e := FormatListOption(forProvider) - if e != nil { - return nil, e - } - r, err = c.Single(ctx, opts) +// FindRouteBySpec looks up a route by spec fields (backwards compatibility). +func (c *Client) FindRouteBySpec(ctx context.Context, forProvider v1alpha1.RouteParameters) (*v1alpha1.RouteObservation, bool, error) { + opts, err := FormatListOption(forProvider) + if err != nil { + return nil, false, err } + r, err := c.Single(ctx, opts) if err != nil { if clients.ErrorIsNotFound(err) { - return nil, nil + return nil, false, nil } - return nil, err + return nil, false, err } - atProvider := GenerateObservation(r) - return &atProvider, nil + return &atProvider, true, nil +} +// GetRouteByGUID fetches a route by its GUID. +func (c *Client) GetRouteByGUID(ctx context.Context, guid string) (*v1alpha1.RouteObservation, bool, error) { + r, err := c.Get(ctx, guid) + if err != nil { + if clients.ErrorIsNotFound(err) { + return nil, false, nil + } + return nil, false, err + } + atProvider := GenerateObservation(r) + return &atProvider, true, nil } // Create creates a Route and returns the GUID or error @@ -86,16 +92,19 @@ func (c *Client) Update(ctx context.Context, guid string, forProvider v1alpha1.R return err } -func (c *Client) Delete(ctx context.Context, guid string) error { +func (c *Client) Delete(ctx context.Context, guid string) (string, error) { if !clients.IsValidGUID(guid) { - return fmt.Errorf("invalid Route GUID") + return "", fmt.Errorf("invalid Route GUID") } - _, err := c.Route.Delete(ctx, guid) + jobGUID, err := c.Route.Delete(ctx, guid) if err != nil { - return err + if clients.ErrorIsNotFound(err) { + return "", nil + } + return "", err } - return nil + return jobGUID, nil } // FormatListOption generates the list options for the client. diff --git a/internal/clients/route/route_test.go b/internal/clients/route/route_test.go index 5998efb3..ef038988 100644 --- a/internal/clients/route/route_test.go +++ b/internal/clients/route/route_test.go @@ -36,23 +36,21 @@ var ( }, URL: &url, } - nilObservation *v1alpha1.RouteObservation - errBoom = errors.New("boom") - errNoResultReturned = client.ErrNoResultsReturned - errExactlyOneResultNotReturned = client.ErrExactlyOneResultNotReturned + errBoom = errors.New("boom") + errNoResultReturned = client.ErrNoResultsReturned ) -func TestGetByIDOrName(t *testing.T) { +func TestFindRouteBySpec(t *testing.T) { type service func() *fake.MockRoute type args struct { - guid string forProvider v1alpha1.RouteParameters } type want struct { - atProvider *v1alpha1.RouteObservation - err error + observation *v1alpha1.RouteObservation + exists bool + err error } cases := map[string]struct { @@ -60,69 +58,114 @@ func TestGetByIDOrName(t *testing.T) { want want service service }{ - "should error when API errors": { + "Found": { args: args{ - guid: guid, forProvider: fakeForProvider, }, want: want{ - atProvider: nilObservation, - err: errBoom, + observation: fakeObservation, + exists: true, + err: nil, }, service: func() *fake.MockRoute { m := &fake.MockRoute{} - m.On("Get", guid).Return( - fake.RouteNil, - errBoom, + m.On("Single").Return( + fake.FakeRoute(guid, url), + nil, ) return m }, }, - "should return nil and ignore error when no result returned": { + "NotFound": { args: args{ - guid: guid, forProvider: fakeForProvider, }, want: want{ - atProvider: nilObservation, - err: nil, + observation: nil, + exists: false, + err: nil, }, service: func() *fake.MockRoute { m := &fake.MockRoute{} - m.On("Get", guid).Return( + m.On("Single").Return( fake.RouteNil, errNoResultReturned, ) return m }, }, - "should return error when exactly one result not returned": { + "Error": { args: args{ - guid: "not-valid", forProvider: fakeForProvider, }, want: want{ - atProvider: nilObservation, - err: nil, + observation: nil, + exists: false, + err: errBoom, }, service: func() *fake.MockRoute { m := &fake.MockRoute{} m.On("Single").Return( fake.RouteNil, - errExactlyOneResultNotReturned, + errBoom, ) return m }, }, + } + + for n, tc := range cases { + t.Run(n, func(t *testing.T) { + c := &Client{ + Route: tc.service(), + } + + obs, exists, err := c.FindRouteBySpec(context.Background(), tc.args.forProvider) - "should get by id": { + if tc.want.err != nil && err != nil { + if diff := cmp.Diff(tc.want.err.Error(), err.Error()); diff != "" { + t.Errorf("FindRouteBySpec(...): want error string != got error string:\n%s", diff) + } + } else { + if diff := cmp.Diff(tc.want.err, err); diff != "" { + t.Errorf("FindRouteBySpec(...): want error != got error:\n%s", diff) + } + } + if diff := cmp.Diff(tc.want.exists, exists); diff != "" { + t.Errorf("FindRouteBySpec(...): -want, +got:\n%s", diff) + } + if diff := cmp.Diff(tc.want.observation, obs); diff != "" { + t.Errorf("FindRouteBySpec(...): -want, +got:\n%s", diff) + } + }) + } +} + +func TestGetRouteByGUID(t *testing.T) { + type service func() *fake.MockRoute + type args struct { + guid string + } + + type want struct { + observation *v1alpha1.RouteObservation + exists bool + err error + } + + cases := map[string]struct { + args args + want want + service service + }{ + "Found": { args: args{ - guid: guid, - forProvider: fakeForProvider, + guid: guid, }, want: want{ - atProvider: fakeObservation, - err: nil, + observation: fakeObservation, + exists: true, + err: nil, }, service: func() *fake.MockRoute { m := &fake.MockRoute{} @@ -133,50 +176,71 @@ func TestGetByIDOrName(t *testing.T) { return m }, }, - "should get by spec": { + "NotFound": { args: args{ - guid: "not-valid", - forProvider: fakeForProvider, + guid: guid, }, want: want{ - atProvider: fakeObservation, - err: nil, + observation: nil, + exists: false, + err: nil, }, service: func() *fake.MockRoute { m := &fake.MockRoute{} - m.On("Single").Return( - fake.FakeRoute(guid, url), - nil, + m.On("Get", guid).Return( + fake.RouteNil, + errNoResultReturned, + ) + return m + }, + }, + "Error": { + args: args{ + guid: guid, + }, + want: want{ + observation: nil, + exists: false, + err: errBoom, + }, + service: func() *fake.MockRoute { + m := &fake.MockRoute{} + m.On("Get", guid).Return( + fake.RouteNil, + errBoom, ) return m }, }, } + for n, tc := range cases { t.Run(n, func(t *testing.T) { - t.Logf("Testing: %s", t.Name()) c := &Client{ Route: tc.service(), } - obs, err := c.GetByIDOrSpec(context.Background(), tc.args.guid, tc.args.forProvider) + obs, exists, err := c.GetRouteByGUID(context.Background(), tc.args.guid) if tc.want.err != nil && err != nil { - // the case where our mock server returns error. if diff := cmp.Diff(tc.want.err.Error(), err.Error()); diff != "" { - t.Errorf("Observe(...): want error string != got error string:\n%s", diff) + t.Errorf("GetRouteByGUID(...): want error string != got error string:\n%s", diff) } } else { if diff := cmp.Diff(tc.want.err, err); diff != "" { - t.Errorf("Observe(...): want error != got error:\n%s", diff) + t.Errorf("GetRouteByGUID(...): want error != got error:\n%s", diff) } } - if diff := cmp.Diff(tc.want.atProvider, obs); diff != "" { - t.Errorf("Observe(...): -want, +got:\n%s", diff) + if diff := cmp.Diff(tc.want.exists, exists); diff != "" { + t.Errorf("GetRouteByGUID(...): -want, +got:\n%s", diff) + } + if diff := cmp.Diff(tc.want.observation, obs); diff != "" { + t.Errorf("GetRouteByGUID(...): -want, +got:\n%s", diff) } }) } } + func TestCreate(t *testing.T) { type service func() *fake.MockRoute type args struct { @@ -252,17 +316,16 @@ func TestCreate(t *testing.T) { id, err := c.Create(context.Background(), tc.args.forProvider) if tc.want.err != nil && err != nil { - // the case where our mock server returns error. if diff := cmp.Diff(tc.want.err.Error(), err.Error()); diff != "" { - t.Errorf("Observe(...): want error string != got error string:\n%s", diff) + t.Errorf("Create(...): want error string != got error string:\n%s", diff) } } else { if diff := cmp.Diff(tc.want.err, err); diff != "" { - t.Errorf("Observe(...): want error != got error:\n%s", diff) + t.Errorf("Create(...): want error != got error:\n%s", diff) } } if diff := cmp.Diff(tc.want.guid, id); diff != "" { - t.Errorf("Observe(...): -want, +got:\n%s", diff) + t.Errorf("Create(...): -want, +got:\n%s", diff) } }) } @@ -275,7 +338,8 @@ func TestDelete(t *testing.T) { } type want struct { - err error + jobGUID string + err error } cases := map[string]struct { @@ -283,47 +347,66 @@ func TestDelete(t *testing.T) { want want service service }{ - "should error when API errors": { + "Successful": { args: args{ guid: guid, }, want: want{ - err: errBoom, + jobGUID: "job-guid-123", + err: nil, + }, + service: func() *fake.MockRoute { + m := &fake.MockRoute{} + m.On("Delete").Return( + "job-guid-123", + nil, + ) + return m + }, + }, + "NotFound": { + args: args{ + guid: guid, + }, + want: want{ + jobGUID: "", + err: nil, }, service: func() *fake.MockRoute { m := &fake.MockRoute{} m.On("Delete").Return( "", - errBoom, + errNoResultReturned, ) return m }, }, - "should error when guid is invalid": { + "InvalidGUID": { args: args{ guid: "not-valid", }, want: want{ - err: errors.New("invalid Route GUID"), + jobGUID: "", + err: errors.New("invalid Route GUID"), }, service: func() *fake.MockRoute { m := &fake.MockRoute{} return m }, }, - - "should delete": { + "Error": { args: args{ guid: guid, }, want: want{ - err: nil, + jobGUID: "", + err: errBoom, }, service: func() *fake.MockRoute { m := &fake.MockRoute{} m.On("Delete").Return( "", - nil, + errBoom, ) return m }, @@ -336,17 +419,20 @@ func TestDelete(t *testing.T) { Route: tc.service(), } - err := c.Delete(context.Background(), tc.args.guid) + jobGUID, err := c.Delete(context.Background(), tc.args.guid) + if tc.want.err != nil && err != nil { - // the case where our mock server returns error. if diff := cmp.Diff(tc.want.err.Error(), err.Error()); diff != "" { - t.Errorf("Observe(...): want error string != got error string:\n%s", diff) + t.Errorf("Delete(...): want error string != got error string:\n%s", diff) } } else { if diff := cmp.Diff(tc.want.err, err); diff != "" { - t.Errorf("Observe(...): want error != got error:\n%s", diff) + t.Errorf("Delete(...): want error != got error:\n%s", diff) } } + if diff := cmp.Diff(tc.want.jobGUID, jobGUID); diff != "" { + t.Errorf("Delete(...): -want, +got:\n%s", diff) + } }) } } diff --git a/internal/controller/route/controller.go b/internal/controller/route/controller.go index dfddce48..43ce5713 100644 --- a/internal/controller/route/controller.go +++ b/internal/controller/route/controller.go @@ -2,6 +2,7 @@ package route import ( "context" + "fmt" "github.com/pkg/errors" @@ -24,16 +25,20 @@ import ( apisv1beta1 "github.com/SAP/crossplane-provider-cloudfoundry/apis/v1beta1" "github.com/SAP/crossplane-provider-cloudfoundry/internal/clients" "github.com/SAP/crossplane-provider-cloudfoundry/internal/clients/domain" + "github.com/SAP/crossplane-provider-cloudfoundry/internal/clients/job" "github.com/SAP/crossplane-provider-cloudfoundry/internal/clients/route" "github.com/SAP/crossplane-provider-cloudfoundry/internal/clients/space" "github.com/SAP/crossplane-provider-cloudfoundry/internal/features" ) type RouteService interface { - GetByIDOrSpec(ctx context.Context, guid string, forProvider v1alpha1.RouteParameters) (*v1alpha1.RouteObservation, error) + FindRouteBySpec(ctx context.Context, forProvider v1alpha1.RouteParameters) (*v1alpha1.RouteObservation, bool, error) + + GetRouteByGUID(ctx context.Context, guid string) (*v1alpha1.RouteObservation, bool, error) + Create(ctx context.Context, forProvider v1alpha1.RouteParameters) (string, error) Update(ctx context.Context, guid string, forProvider v1alpha1.RouteParameters) error - Delete(ctx context.Context, guid string) error + Delete(ctx context.Context, guid string) (string, error) } const ( @@ -114,7 +119,8 @@ func (c *connector) Connect(ctx context.Context, mg resource.Managed) (managed.E return nil, errors.Wrap(err, errNewClient) } - return &external{RouteService: route.NewClient(cf), kube: c.kube}, nil + routeClient, jobClient := route.NewClient(cf) + return &external{RouteService: routeClient, kube: c.kube, job: jobClient}, nil } // Disconnect implements the managed.ExternalClient interface @@ -126,10 +132,9 @@ func (c *external) Disconnect(ctx context.Context) error { // An ExternalClient observes, then either creates, updates, or deletes an // external resource to ensure it reflects the managed resource's desired state. type external struct { - // A 'client' used to connect to the external resource API. In practice this - // would be something like an AWS SDK client. kube k8s.Client RouteService + job job.Job } // Observe generates observation for Route's @@ -139,33 +144,43 @@ func (c *external) Observe(ctx context.Context, mg resource.Managed) (managed.Ex return managed.ExternalObservation{}, errors.New(errNotRoute) } + resourceLateInitialized := false + + if meta.GetExternalName(cr) == "" { + // Backwards compatibility: lookup by spec fields + observed, exists, err := c.FindRouteBySpec(ctx, cr.Spec.ForProvider) + if err != nil { + return managed.ExternalObservation{}, errors.Wrap(err, errGet) + } + if !exists { + return managed.ExternalObservation{ResourceExists: false}, nil + } + meta.SetExternalName(cr, observed.GUID) + resourceLateInitialized = true + } + guid := meta.GetExternalName(cr) - atProvider, err := c.GetByIDOrSpec(ctx, guid, cr.Spec.ForProvider) + if !clients.IsValidGUID(guid) { + return managed.ExternalObservation{}, errors.New(fmt.Sprintf("external-name '%s' is not a valid GUID format", guid)) + } + + observed, exists, err := c.GetRouteByGUID(ctx, guid) if err != nil { return managed.ExternalObservation{}, errors.Wrap(err, errGet) } - - if atProvider == nil { + if !exists { return managed.ExternalObservation{ResourceExists: false}, nil } - cr.SetConditions(xpv1.Available()) - - lateInitialized := false - if atProvider.GUID != guid { - meta.SetExternalName(cr, atProvider.GUID) - lateInitialized = true - } - - cr.Status.AtProvider = *atProvider + cr.Status.AtProvider = *observed + cr.Status.SetConditions(xpv1.Available()) return managed.ExternalObservation{ ResourceExists: true, - ResourceUpToDate: route.IsUpToDate(cr.Spec.ForProvider, *atProvider), - ResourceLateInitialized: lateInitialized, + ResourceUpToDate: route.IsUpToDate(cr.Spec.ForProvider, *observed), + ResourceLateInitialized: resourceLateInitialized, }, nil - } // Create a route @@ -185,8 +200,6 @@ func (c *external) Create(ctx context.Context, mg resource.Managed) (managed.Ext meta.SetExternalName(cr, guid) return managed.ExternalCreation{ - // Optionally return any details that may be required to connect to the - // external resource. These will be stored as the connection secret. ConnectionDetails: managed.ConnectionDetails{}, }, nil } @@ -225,8 +238,16 @@ func (c *external) Delete(ctx context.Context, mg resource.Managed) (managed.Ext cr.SetConditions(xpv1.Deleting()) - return managed.ExternalDelete{}, c.RouteService.Delete(ctx, meta.GetExternalName(cr)) + if meta.GetExternalName(cr) == "" { + return managed.ExternalDelete{}, nil + } + + jobGUID, err := c.RouteService.Delete(ctx, meta.GetExternalName(cr)) + if err != nil { + return managed.ExternalDelete{}, errors.Wrap(err, errDelete) + } + return managed.ExternalDelete{}, job.PollJobComplete(ctx, c.job, jobGUID) } // ResolveReferences of this Route. diff --git a/internal/controller/route/controller_test.go b/internal/controller/route/controller_test.go index 15bf9f56..97e111bc 100644 --- a/internal/controller/route/controller_test.go +++ b/internal/controller/route/controller_test.go @@ -4,6 +4,7 @@ import ( "context" "testing" + xpv1 "github.com/crossplane/crossplane-runtime/apis/common/v1" "github.com/crossplane/crossplane-runtime/pkg/resource" "github.com/google/go-cmp/cmp" "github.com/pkg/errors" @@ -16,6 +17,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "github.com/SAP/crossplane-provider-cloudfoundry/apis/resources/v1alpha1" + "github.com/SAP/crossplane-provider-cloudfoundry/internal/clients/fake" ) // Mock mocks RouteService interface @@ -23,9 +25,14 @@ type Mock struct { mock.Mock } -func (m *Mock) GetByIDOrSpec(ctx context.Context, guid string, forProvider v1alpha1.RouteParameters) (*v1alpha1.RouteObservation, error) { +func (m *Mock) FindRouteBySpec(ctx context.Context, forProvider v1alpha1.RouteParameters) (*v1alpha1.RouteObservation, bool, error) { + args := m.Called(forProvider) + return args.Get(0).(*v1alpha1.RouteObservation), args.Bool(1), args.Error(2) +} + +func (m *Mock) GetRouteByGUID(ctx context.Context, guid string) (*v1alpha1.RouteObservation, bool, error) { args := m.Called(guid) - return args.Get(0).(*v1alpha1.RouteObservation), args.Error(1) + return args.Get(0).(*v1alpha1.RouteObservation), args.Bool(1), args.Error(2) } func (m *Mock) Create(ctx context.Context, forProvider v1alpha1.RouteParameters) (string, error) { @@ -38,25 +45,26 @@ func (m *Mock) Update(ctx context.Context, guid string, forProvider v1alpha1.Rou return args.Error(0) } -func (m *Mock) Delete(ctx context.Context, guid string) error { - args := m.Called() - return args.Error(0) +func (m *Mock) Delete(ctx context.Context, guid string) (string, error) { + args := m.Called(guid) + return args.String(0), args.Error(1) } var ( - spaceGUID = "11fd5b0b-4f3b-4b1b-8b3d-3b5f7b4b3b4b" - domainGUID = "22fd5b0b-4f3b-4b1b-8b3d-3b5f7b4b3b4b" - guid = "33fd5b0b-4f3b-4b1b-8b3d-3b5f7b4b3b4b" - name = "test-route" - errBoom = errors.New("boom") + spaceGUID = "11fd5b0b-4f3b-4b1b-8b3d-3b5f7b4b3b4b" + domainGUID = "22fd5b0b-4f3b-4b1b-8b3d-3b5f7b4b3b4b" + guid = "33fd5b0b-4f3b-4b1b-8b3d-3b5f7b4b3b4b" + name = "test-route" + errBoom = errors.New("boom") + nilObservation *v1alpha1.RouteObservation ) type modifier func(*v1alpha1.Route) -func withExternalName(guid string) modifier { +func withExternalName(externalName string) modifier { return func(r *v1alpha1.Route) { - r.Annotations[meta.AnnotationKeyExternalName] = guid + r.Annotations[meta.AnnotationKeyExternalName] = externalName } } @@ -66,6 +74,16 @@ func withHost(host string) modifier { } } +func withConditions(c ...xpv1.Condition) modifier { + return func(r *v1alpha1.Route) { r.Status.SetConditions(c...) } +} + +func withDestinations(destinations []v1alpha1.RouteDestination) modifier { + return func(r *v1alpha1.Route) { + r.Status.AtProvider.Destinations = destinations + } +} + func fakeRoute(m ...modifier) *v1alpha1.Route { r := &v1alpha1.Route{ ObjectMeta: metav1.ObjectMeta{ @@ -104,7 +122,6 @@ func TestObserve(t *testing.T) { } type want struct { - mg *v1alpha1.Route obs managed.ExternalObservation err error } @@ -115,13 +132,12 @@ func TestObserve(t *testing.T) { service service kube k8s.Client }{ - "Error if mg is not the right kind": { + "Nil": { args: args{ mg: nil, }, want: want{ - mg: nil, - obs: managed.ExternalObservation{ResourceExists: false}, + obs: managed.ExternalObservation{}, err: errors.New(errNotRoute), }, service: func() *Mock { @@ -129,107 +145,104 @@ func TestObserve(t *testing.T) { return m }, }, - // This tests whether the external API is reachable - "Error when external API is not working": { + "UnsetExternalNameSuccessful": { args: args{ - mg: fakeRoute(withExternalName(guid)), + mg: fakeRoute(withHost(name)), }, want: want{ - mg: fakeRoute(withExternalName(guid)), - obs: managed.ExternalObservation{}, - err: errors.Wrap(errBoom, errGet), + obs: managed.ExternalObservation{ + ResourceExists: true, + ResourceUpToDate: true, + ResourceLateInitialized: true, + }, + err: nil, }, service: func() *Mock { m := &Mock{} - m.On("GetByIDOrSpec", guid).Return( - nilObservation, - errBoom, + m.On("FindRouteBySpec", fakeRoute(withHost(name)).Spec.ForProvider).Return( + fakeRouteObservation(guid), true, nil, + ) + m.On("GetRouteByGUID", guid).Return( + fakeRouteObservation(guid), true, nil, ) return m }, }, - "NotFound (guid) with nil observation ": { + "UnsetExternalNameNotFound": { args: args{ - mg: fakeRoute(withExternalName(guid)), + mg: fakeRoute(withHost(name)), }, want: want{ - mg: fakeRoute(withExternalName(guid)), - obs: managed.ExternalObservation{ResourceExists: false, ResourceLateInitialized: false}, + obs: managed.ExternalObservation{ResourceExists: false}, err: nil, }, service: func() *Mock { m := &Mock{} - m.On("GetByIDOrSpec", guid).Return( - nilObservation, - nil, + m.On("FindRouteBySpec", fakeRoute(withHost(name)).Spec.ForProvider).Return( + nilObservation, false, nil, ) return m }, - kube: &test.MockClient{}, }, - "NotFound if nil observation is returned": { + "SetExternalNameSuccessful": { args: args{ - mg: fakeRoute(withHost(name)), + mg: fakeRoute( + withExternalName(guid), + withHost(name), + ), }, want: want{ - mg: fakeRoute(withHost(name), withExternalName(guid)), - obs: managed.ExternalObservation{ - ResourceExists: false, - }, + obs: managed.ExternalObservation{ResourceExists: true, ResourceUpToDate: true}, err: nil, }, service: func() *Mock { m := &Mock{} - m.On("GetByIDOrSpec", "").Return( // this should be called - nilObservation, - nil, + m.On("GetRouteByGUID", guid).Return( + fakeRouteObservation(guid), true, nil, ) return m }, }, - "Found with observation is returned": { + "SetExternalNameNotFound": { args: args{ - mg: fakeRoute( - withExternalName(guid), - withHost(name), - ), + mg: fakeRoute(withExternalName(guid)), }, want: want{ - mg: fakeRoute( - withExternalName(guid), - withHost(name), - ), - obs: managed.ExternalObservation{ResourceExists: true, ResourceUpToDate: true}, + obs: managed.ExternalObservation{ResourceExists: false}, err: nil, }, service: func() *Mock { m := &Mock{} - - m.On("GetByIDOrSpec", guid).Return( - fakeRouteObservation(guid), - nil, + m.On("GetRouteByGUID", guid).Return( + nilObservation, false, nil, ) return m }, }, - "Adopt and set external-name ": { + "SetExternalNameInvalidFormat": { args: args{ - mg: fakeRoute(withHost(name)), + mg: fakeRoute(withExternalName("not-a-valid-guid")), }, want: want{ - mg: fakeRoute(withHost(name), withExternalName(guid)), - obs: managed.ExternalObservation{ - ResourceExists: true, - ResourceUpToDate: true, - ResourceLateInitialized: true, - }, - err: nil, + obs: managed.ExternalObservation{}, + err: errors.New("external-name 'not-a-valid-guid' is not a valid GUID format"), + }, + service: func() *Mock { + return &Mock{} + }, + }, + "Error": { + args: args{ + mg: fakeRoute(withExternalName(guid)), + }, + want: want{ + obs: managed.ExternalObservation{}, + err: errors.Wrap(errBoom, errGet), }, service: func() *Mock { m := &Mock{} - m.On("GetByIDOrSpec", "").Return( - fakeRouteObservation(guid), - nil, + m.On("GetRouteByGUID", guid).Return( + nilObservation, false, errBoom, ) return m }, @@ -246,11 +259,6 @@ func TestObserve(t *testing.T) { } obs, err := c.Observe(context.Background(), tc.args.mg) - var Domain *v1alpha1.Domain - if tc.args.mg != nil { - Domain, _ = tc.args.mg.(*v1alpha1.Domain) - } - if tc.want.err != nil && err != nil { // the case where our mock server returns error. if diff := cmp.Diff(tc.want.err.Error(), err.Error()); diff != "" { @@ -264,11 +272,207 @@ func TestObserve(t *testing.T) { if diff := cmp.Diff(tc.want.obs, obs); diff != "" { t.Errorf("Observe(...): -want, +got:\n%s", diff) } - if Domain != nil && tc.want.mg != nil { + }) + } +} + +func TestCreate(t *testing.T) { + type service func() *Mock + type args struct { + mg resource.Managed + } + + type want struct { + obs managed.ExternalCreation + err error + } + + cases := map[string]struct { + args args + want want + service service + kube k8s.Client + }{ + "Successful": { + args: args{ + mg: fakeRoute(), + }, + want: want{ + obs: managed.ExternalCreation{ConnectionDetails: managed.ConnectionDetails{}}, + err: nil, + }, + service: func() *Mock { + m := &Mock{} + m.On("Create").Return(guid, nil) + return m + }, + }, + "AlreadyExist": { + args: args{ + mg: fakeRoute(), + }, + want: want{ + obs: managed.ExternalCreation{}, + err: errors.Wrap(errBoom, errCreate), + }, + service: func() *Mock { + m := &Mock{} + m.On("Create").Return("", errBoom) + return m + }, + }, + } + + for n, tc := range cases { + t.Run(n, func(t *testing.T) { + c := &external{ + kube: &test.MockClient{ + MockUpdate: test.NewMockUpdateFn(nil), + MockStatusUpdate: test.NewMockSubResourceUpdateFn(nil), + }, + RouteService: tc.service(), + } + + obs, err := c.Create(context.Background(), tc.args.mg) + + if tc.want.err != nil && err != nil { + if diff := cmp.Diff(tc.want.err.Error(), err.Error()); diff != "" { + t.Errorf("Create(...): want error string != got error string:\n%s", diff) + } + } else { + if diff := cmp.Diff(tc.want.err, err); diff != "" { + t.Errorf("Create(...): want error != got error:\n%s", diff) + } + } + if diff := cmp.Diff(tc.want.obs, obs); diff != "" { + t.Errorf("Create(...): -want, +got:\n%s", diff) + } + }) + } +} + +func TestDelete(t *testing.T) { + type service func() *Mock + type args struct { + mg resource.Managed + } + + type want struct { + mg resource.Managed + obs managed.ExternalDelete + err error + } + + cases := map[string]struct { + args args + want want + service service + kube k8s.Client + }{ + "SuccessfulDelete": { + args: args{ + mg: fakeRoute(withExternalName(guid)), + }, + want: want{ + mg: fakeRoute(withExternalName(guid), withConditions(xpv1.Deleting())), + obs: managed.ExternalDelete{}, + err: nil, + }, + service: func() *Mock { + m := &Mock{} + m.On("Delete", guid).Return("job-guid-123", nil) + return m + }, + }, + "404NotFound": { + args: args{ + mg: fakeRoute(withExternalName(guid)), + }, + want: want{ + mg: fakeRoute(withExternalName(guid), withConditions(xpv1.Deleting())), + obs: managed.ExternalDelete{}, + err: nil, + }, + service: func() *Mock { + m := &Mock{} + m.On("Delete", guid).Return("", nil) + return m + }, + }, + "Error": { + args: args{ + mg: fakeRoute(withExternalName(guid)), + }, + want: want{ + mg: fakeRoute(withExternalName(guid), withConditions(xpv1.Deleting())), + obs: managed.ExternalDelete{}, + err: errors.Wrap(errBoom, errDelete), + }, + service: func() *Mock { + m := &Mock{} + m.On("Delete", guid).Return("", errBoom) + return m + }, + }, + "EmptyExternalName": { + args: args{ + mg: fakeRoute(), + }, + want: want{ + mg: fakeRoute(withConditions(xpv1.Deleting())), + obs: managed.ExternalDelete{}, + err: nil, + }, + service: func() *Mock { + m := &Mock{} + return m + }, + }, + "ActiveBindings": { + args: args{ + mg: fakeRoute(withExternalName(guid), withDestinations([]v1alpha1.RouteDestination{{GUID: "dest-guid"}})), + }, + want: want{ + mg: fakeRoute(withExternalName(guid), withDestinations([]v1alpha1.RouteDestination{{GUID: "dest-guid"}})), + obs: managed.ExternalDelete{}, + err: errors.New(errActiveBinding), + }, + service: func() *Mock { + m := &Mock{} + return m + }, + }, + } + + for n, tc := range cases { + t.Run(n, func(t *testing.T) { + mockJob := &fake.MockJob{} + mockJob.On("PollComplete").Return(nil) + + c := &external{ + kube: &test.MockClient{ + MockDelete: test.NewMockDeleteFn(nil), + }, + job: mockJob, + RouteService: tc.service(), + } - if diff := cmp.Diff(Domain.Spec, tc.want.mg.Spec); diff != "" { - t.Errorf("Observe(...): -want, +got:\n%s", diff) + obs, err := c.Delete(context.Background(), tc.args.mg) + + if tc.want.err != nil && err != nil { + if diff := cmp.Diff(tc.want.err.Error(), err.Error()); diff != "" { + t.Errorf("Delete(...): want error string != got error string:\n%s", diff) } + } else { + if diff := cmp.Diff(tc.want.err, err); diff != "" { + t.Errorf("Delete(...): want error != got error:\n%s", diff) + } + } + if diff := cmp.Diff(tc.want.obs, obs); diff != "" { + t.Errorf("Delete(...): -want, +got:\n%s", diff) + } + if diff := cmp.Diff(tc.want.mg, tc.args.mg); diff != "" { + t.Errorf("Delete(...): -want, +got:\n%s", diff) } }) } diff --git a/test/e2e/cloudfoundry_route_import_test.go b/test/e2e/cloudfoundry_route_import_test.go new file mode 100644 index 00000000..67b9fb1b --- /dev/null +++ b/test/e2e/cloudfoundry_route_import_test.go @@ -0,0 +1,46 @@ +//go:build e2e + +package e2e + +import ( + "testing" + "time" + + "github.com/SAP/crossplane-provider-cloudfoundry/apis/resources/v1alpha1" + "sigs.k8s.io/e2e-framework/klient/wait" +) + +var ( + routeImportTestK8sResName = "e2e-test-route-import" + routeImportTestDomainName = "cfapps.eu12.hana.ondemand.com" + routeImportTestSpaceName = "import-test-space-donotdelete" + routeImportTestOrgName = "cf-ci-e2e" + routeImportTestHost = "route-import-e2e" +) + +func TestRouteImportFlow(t *testing.T) { + importTester := NewImportTester( + &v1alpha1.Route{ + Spec: v1alpha1.RouteSpec{ + ForProvider: v1alpha1.RouteParameters{ + DomainReference: v1alpha1.DomainReference{ + DomainName: &routeImportTestDomainName, + }, + SpaceReference: v1alpha1.SpaceReference{ + SpaceName: &routeImportTestSpaceName, + OrgName: &routeImportTestOrgName, + }, + Host: &routeImportTestHost, + }, + }, + }, + routeImportTestK8sResName, + WithDependentResourceDirectory[*v1alpha1.Route]("./crs/route"), + WithWaitCreateTimeout[*v1alpha1.Route](wait.WithTimeout(5*time.Minute)), + WithWaitDeletionTimeout[*v1alpha1.Route](wait.WithTimeout(5*time.Minute)), + ) + + importFeature := importTester.BuildTestFeature("CF Route Import Flow").Feature() + + testenv.Test(t, importFeature) +} diff --git a/test/e2e/crs/route/domain.yaml b/test/e2e/crs/route/domain.yaml new file mode 100644 index 00000000..b35360a1 --- /dev/null +++ b/test/e2e/crs/route/domain.yaml @@ -0,0 +1,14 @@ +apiVersion: cloudfoundry.crossplane.io/v1alpha1 +kind: Domain +metadata: + name: e2e-route-import-domain +spec: + managementPolicies: + - Observe + forProvider: + name: cfapps.eu12.hana.ondemand.com + orgRef: + name: e2e-route-import-org + policy: + resolve: Always + internal: false diff --git a/test/e2e/crs/route/import.yaml b/test/e2e/crs/route/import.yaml new file mode 100644 index 00000000..e9549642 --- /dev/null +++ b/test/e2e/crs/route/import.yaml @@ -0,0 +1,26 @@ +--- +apiVersion: cloudfoundry.crossplane.io/v1alpha1 +kind: Organization +metadata: + name: e2e-route-import-org +spec: + managementPolicies: + - Observe + forProvider: + # change to your testing org + name: cf-ci-e2e +--- +# import space for route +apiVersion: cloudfoundry.crossplane.io/v1alpha1 +kind: Space +metadata: + name: e2e-route-import-space +spec: + managementPolicies: + - Observe + forProvider: + name: import-test-space-donotdelete + orgRef: + name: e2e-route-import-org + policy: + resolve: Always diff --git a/test/e2e/crs/route/route.yaml b/test/e2e/crs/route/route.yaml new file mode 100644 index 00000000..e5c25f26 --- /dev/null +++ b/test/e2e/crs/route/route.yaml @@ -0,0 +1,15 @@ +apiVersion: cloudfoundry.crossplane.io/v1alpha1 +kind: Route +metadata: + name: e2e-route-import-test +spec: + forProvider: + domainRef: + name: e2e-route-import-domain + policy: + resolve: Always + host: route-import-e2e + spaceRef: + name: e2e-route-import-space + policy: + resolve: Always diff --git a/test/upgrade/route_external_name_upgrade_test.go b/test/upgrade/route_external_name_upgrade_test.go new file mode 100644 index 00000000..446ecd81 --- /dev/null +++ b/test/upgrade/route_external_name_upgrade_test.go @@ -0,0 +1,111 @@ +//go:build upgrade + +// +// This file (route_external_name_upgrade_test.go) contains Test_Route_External_Name, +// which validates that Route resources maintain proper external-name formatting +// during provider upgrades. Specifically, it verifies: +// - External-name annotation exists and follows UUID format +// - External-name value remains unchanged after provider upgrade + +package upgrade + +import ( + "context" + "testing" + + v1alpha1 "github.com/SAP/crossplane-provider-cloudfoundry/apis/resources/v1alpha1" + "github.com/SAP/crossplane-provider-cloudfoundry/test" + "k8s.io/klog/v2" + res "sigs.k8s.io/e2e-framework/klient/k8s/resources" + "sigs.k8s.io/e2e-framework/pkg/envconf" +) + +var ( + routeCustomResourceDirectories = []string{ + "./testdata/customCrs/externalNames/import", + "./testdata/customCrs/externalNames/route", + } +) + +func Test_Route_External_Name(t *testing.T) { + const routeName = "upgrade-test-route" + + upgradeTest := NewCustomUpgradeTest("route-external-name-test"). + FromVersion(fromTag). + ToVersion(toTag). + WithResourceDirectories(routeCustomResourceDirectories). + WithCustomPreUpgradeAssessment( + "Verify external name before upgrade", + func(ctx context.Context, t *testing.T, cfg *envconf.Config) context.Context { + r, err := res.New(cfg.Client().RESTConfig()) + if err != nil { + t.Fatalf("Failed to create resource client: %v", err) + } + + err = v1alpha1.SchemeBuilder.AddToScheme(r.GetScheme()) + if err != nil { + t.Fatalf("Failed to add CloudFoundry scheme: %v", err) + } + + route := &v1alpha1.Route{} + + err = r.Get(ctx, routeName, cfg.Namespace(), route) + if err != nil { + t.Fatalf("Failed to get Route resource: %v", err) + } + + annotations := route.GetAnnotations() + externalName, exists := annotations["crossplane.io/external-name"] + if !exists { + t.Fatal("External name annotation does not exist") + } + + klog.V(4).Infof("Pre-upgrade external name: %s", externalName) + + if !test.UUIDRegex.MatchString(externalName) { + t.Fatalf("External name '%s' does not match expected UUID format", externalName) + } + + return context.WithValue(ctx, "preUpgradeRouteExternalName", externalName) + }, + ). + WithCustomPostUpgradeAssessment( + "Verify external name after upgrade", + func(ctx context.Context, t *testing.T, cfg *envconf.Config) context.Context { + route := &v1alpha1.Route{} + r := cfg.Client().Resources() + + err := r.Get(ctx, routeName, cfg.Namespace(), route) + if err != nil { + t.Fatalf("Failed to get Route resource: %v", err) + } + + annotations := route.GetAnnotations() + externalName, exists := annotations["crossplane.io/external-name"] + if !exists { + t.Fatal("External name annotation does not exist after upgrade") + } + + klog.V(4).Infof("Post-upgrade external name: %s", externalName) + + if !test.UUIDRegex.MatchString(externalName) { + t.Fatalf("External name '%s' does not match expected UUID format after upgrade", externalName) + } + + preUpgradeExternalName, ok := ctx.Value("preUpgradeRouteExternalName").(string) + if !ok { + t.Fatal("Failed to retrieve pre-upgrade external name from context") + } + + if externalName != preUpgradeExternalName { + t.Fatalf("External name changed during upgrade: before='%s', after='%s'", + preUpgradeExternalName, externalName) + } + + klog.V(4).Info("External name validation passed: format correct and unchanged") + return ctx + }, + ) + + testenv.Test(t, upgradeTest.Feature()) +} diff --git a/test/upgrade/testdata/customCrs/externalNames/route/route.yaml b/test/upgrade/testdata/customCrs/externalNames/route/route.yaml new file mode 100644 index 00000000..3414173a --- /dev/null +++ b/test/upgrade/testdata/customCrs/externalNames/route/route.yaml @@ -0,0 +1,19 @@ +apiVersion: cloudfoundry.crossplane.io/v1alpha1 +kind: Route +metadata: + name: upgrade-test-route + annotations: + crossplane.io/paused: "false" +spec: + forProvider: + domainRef: + name: upgrade-test-domain + policy: + resolve: Always + host: upgrade-test-route-host + spaceRef: + name: upgrade-test-import-space + policy: + resolve: Always + providerConfigRef: + name: default