diff --git a/.github/workflows/e2e_test.yaml b/.github/workflows/e2e_test.yaml index a0d943ee..8f241140 100644 --- a/.github/workflows/e2e_test.yaml +++ b/.github/workflows/e2e_test.yaml @@ -66,10 +66,14 @@ jobs: - name: Install gettext for envsubst run: sudo apt-get update && sudo apt-get install -y gettext + - name: Set BUILD_ID + run: echo "BUILD_ID=${{ github.run_number }}" >> $GITHUB_ENV + - name: Run build and e2e test run: make test-acceptance env: CF_CREDENTIALS: ${{ secrets.CF_CREDENTIALS }} CF_ENVIRONMENT: ${{ secrets.CF_ENVIRONMENT }} + BUILD_ID: ${{ env.BUILD_ID }} # USER_WHERE_ROLES_GET_ASSIGNED_EMAIL: ${{ secrets.USER_WHERE_ROLES_GET_ASSIGNED_EMAIL }} # TECHNICAL_USER_EMAIL: ${{ secrets.TECHNICAL_USER_EMAIL }} diff --git a/apis/resources/v1alpha1/spacerole_types.go b/apis/resources/v1alpha1/spacerole_types.go index 82ba8196..92021ea1 100755 --- a/apis/resources/v1alpha1/spacerole_types.go +++ b/apis/resources/v1alpha1/spacerole_types.go @@ -71,6 +71,14 @@ type SpaceRoleStatus struct { // +kubebuilder:storageversion // SpaceRole is the Schema for the OrgRoles API. Provides a Cloud Foundry resource for assigning org roles.(Updating a role is not supported according to the docs) +// +// External-Name Configuration: +// - Follows Standard: yes +// - Format: Space Role GUID (UUID format) +// - How to find: +// - UI: Not available in the BTP Cockpit +// - CLI: Use CF CLI: `cf space-users -v` and find the GUID in the output +// // +kubebuilder:printcolumn:name="SYNCED",type="string",JSONPath=".status.conditions[?(@.type=='Synced')].status" // +kubebuilder:printcolumn:name="READY",type="string",JSONPath=".status.conditions[?(@.type=='Ready')].status" // +kubebuilder:printcolumn:name="EXTERNAL-NAME",type="string",JSONPath=".metadata.annotations.crossplane\\.io/external-name" diff --git a/internal/clients/role/spacerole.go b/internal/clients/role/spacerole.go index 8f17a687..c0ca0e08 100644 --- a/internal/clients/role/spacerole.go +++ b/internal/clients/role/spacerole.go @@ -11,7 +11,6 @@ import ( "github.com/cloudfoundry/go-cfclient/v3/resource" "github.com/SAP/crossplane-provider-cloudfoundry/apis/resources/v1alpha1" - "github.com/SAP/crossplane-provider-cloudfoundry/internal/clients" ) const ErrSpaceNotSpecified = "space is not specified" @@ -33,15 +32,12 @@ func SpaceRoleType(roleType string) resource.SpaceRoleType { } // GetSpaceRole returns the role of a user in a space by guid or by matching the spec -func GetSpaceRole(ctx context.Context, client Role, guid string, spec v1alpha1.SpaceRoleParameters) (*resource.Role, error) { - if clients.IsValidGUID(guid) { - return client.Get(ctx, guid) - } - return findSpaceRole(ctx, client, spec) +func GetSpaceRole(ctx context.Context, client Role, guid string) (*resource.Role, error) { + return client.Get(ctx, guid) } // searchSpaceRole returns the role of a user in a space if the role matches the spec -func findSpaceRole(ctx context.Context, client Role, spec v1alpha1.SpaceRoleParameters) (*resource.Role, error) { +func FindSpaceRole(ctx context.Context, client Role, spec v1alpha1.SpaceRoleParameters) (*resource.Role, error) { opt, err := newSpaceRoleListOptions(spec) if err != nil { diff --git a/internal/controller/spacerole/controller.go b/internal/controller/spacerole/controller.go index 027c52df..e8e16b6d 100644 --- a/internal/controller/spacerole/controller.go +++ b/internal/controller/spacerole/controller.go @@ -2,6 +2,7 @@ package spacerole import ( "context" + "fmt" "github.com/pkg/errors" @@ -125,9 +126,33 @@ func (c *external) Observe(ctx context.Context, mg resource.Managed) (managed.Ex return managed.ExternalObservation{}, errors.New(errWrongKind) } - // Fetch the role object using the CloudFoundry API by guid or according to the specified parameters + resourceLateInitialized := false + + // ADR Step 1: Check if external-name is empty + if meta.GetExternalName(cr) == "" { + // Backwards compatibility required as previously lookup by spec was supported + r, err := role.FindSpaceRole(ctx, c.role, cr.Spec.ForProvider) + if err != nil { + if clients.ErrorIsNotFound(err) { + return managed.ExternalObservation{ResourceExists: false}, nil + } + return managed.ExternalObservation{}, errors.Wrap(err, errGet) + } + if r == nil { + return managed.ExternalObservation{ResourceExists: false}, nil + } + meta.SetExternalName(cr, r.GUID) + resourceLateInitialized = true + } + guid := meta.GetExternalName(cr) - r, err := role.GetSpaceRole(ctx, c.role, guid, cr.Spec.ForProvider) + + // ADR Step 2: External-name is set, check its format (must be valid GUID) + if !clients.IsValidGUID(guid) { + return managed.ExternalObservation{}, errors.New(fmt.Sprintf("external-name '%s' is not a valid GUID format", guid)) + } + // ADR Step 3: Build the Get API Request from the external-name (using GUID directly) + r, err := role.GetSpaceRole(ctx, c.role, guid) if err != nil { if clients.ErrorIsNotFound(err) { @@ -140,17 +165,11 @@ func (c *external) Observe(ctx context.Context, mg resource.Managed) (managed.Ex return managed.ExternalObservation{ResourceExists: false}, nil } - resourceLateInitialized := false - if guid != r.GUID { - meta.SetExternalName(cr, r.GUID) - resourceLateInitialized = true - } - cr.Status.AtProvider = role.GenerateSpaceRoleObservation(r) cr.Status.SetConditions(xpv1.Available()) return managed.ExternalObservation{ - ResourceExists: cr.Status.AtProvider.ID != nil, + ResourceExists: meta.GetExternalName(cr) != "", ResourceUpToDate: true, ResourceLateInitialized: resourceLateInitialized, }, nil @@ -207,13 +226,17 @@ func (c *external) Delete(ctx context.Context, mg resource.Managed) (managed.Ext // TODO cr.SetConditions(xpv1.Deleting()) - if cr.Status.AtProvider.ID == nil { + if meta.GetExternalName(cr) == "" { return managed.ExternalDelete{}, nil } // Delete is async and we need to implement wait for deletion - jobGUID, err := c.role.Delete(ctx, *cr.Status.AtProvider.ID) + jobGUID, err := c.role.Delete(ctx, meta.GetExternalName(cr)) if err != nil { + // ADR: 404 not found means already deleted - not considered as error case + if clients.ErrorIsNotFound(err) { + return managed.ExternalDelete{}, nil + } return managed.ExternalDelete{}, errors.Wrap(err, errDelete) } diff --git a/internal/controller/spacerole/controller_test.go b/internal/controller/spacerole/controller_test.go index c6fdc9b0..454dbbab 100644 --- a/internal/controller/spacerole/controller_test.go +++ b/internal/controller/spacerole/controller_test.go @@ -5,6 +5,7 @@ import ( "testing" cfresource "github.com/cloudfoundry/go-cfclient/v3/resource" + 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" @@ -88,6 +89,16 @@ func withExternalName(name string) modifier { } } +func withID(id string) modifier { + return func(r *v1alpha1.SpaceRole) { + r.Status.AtProvider.ID = ptr.To(id) + } +} + +func withConditions(c ...xpv1.Condition) modifier { + return func(i *v1alpha1.SpaceRole) { i.Status.SetConditions(c...) } +} + func fakeSpaceRole(m ...modifier) *v1alpha1.SpaceRole { r := &v1alpha1.SpaceRole{ ObjectMeta: metav1.ObjectMeta{ @@ -183,46 +194,49 @@ func TestObserve(t *testing.T) { return m }, }, - "NotFound by uuid is not found": { + "UnsetExternalNameSuccesful": { args: args{ - mg: fakeSpaceRole(withSpace("my-space"), withExternalName(guidRole)), + mg: fakeSpaceRole(withSpace("my-space"), withUsername("user1"), withType(v1alpha1.SpaceManager)), }, want: want{ - mg: fakeSpaceRole(withSpace("my-space"), withExternalName(guidRole)), - obs: managed.ExternalObservation{ResourceExists: false, ResourceUpToDate: false, ResourceLateInitialized: false}, + mg: fakeSpaceRole(withSpace("my-space"), withUsername("user1"), withType(v1alpha1.SpaceManager), withExternalName(guidRole)), + obs: managed.ExternalObservation{ResourceExists: true, ResourceUpToDate: true, ResourceLateInitialized: true}, err: nil, }, service: func() *fake.MockSpaceRole { m := &fake.MockSpaceRole{} + m.On("ListIncludeUsersAll").Return( + []*cfresource.Role{healthyRole}, + []*cfresource.User{healthyUser}, + nil, + ) + m.On("Get", guidRole).Return( - fake.SpaceRoleNil, + healthyRole, nil, ) return m }, }, - "Successful": { + "UnsetExternalNameNotFound": { args: args{ - mg: fakeSpaceRole(withSpace("my-space"), withUsername("user1"), withType(v1alpha1.SpaceManager)), + mg: fakeSpaceRole(withSpace(guidSpace), withUsername("user1"), withType(v1alpha1.SpaceManager)), }, want: want{ - mg: fakeSpaceRole(withSpace("my-space"), withUsername("user1"), withType(v1alpha1.SpaceManager), withExternalName(guidRole)), - obs: managed.ExternalObservation{ResourceExists: true, ResourceUpToDate: true, ResourceLateInitialized: true}, + mg: fakeSpaceRole(withSpace(guidSpace), withUsername("user1"), withType(v1alpha1.SpaceManager)), + obs: managed.ExternalObservation{ResourceExists: false}, err: nil, }, service: func() *fake.MockSpaceRole { m := &fake.MockSpaceRole{} - - m.On("ListIncludeUsersAll").Return( - []*cfresource.Role{healthyRole}, - []*cfresource.User{healthyUser}, - nil, - ) + var emptyRoles []*cfresource.Role + var emptyUsers []*cfresource.User + m.On("ListIncludeUsersAll").Return(emptyRoles, emptyUsers, nil) return m }, }, - "Successful when SpaceRole guid is found": { + "SetExternalNameSuccesful": { args: args{ mg: fakeSpaceRole(withSpace("my-space"), withUsername("user1"), withType(v1alpha1.SpaceManager), withExternalName(guidRole)), }, @@ -241,6 +255,37 @@ func TestObserve(t *testing.T) { return m }, }, + "SetExternalNameNotFound": { + args: args{ + mg: fakeSpaceRole(withExternalName(guidRole)), + }, + want: want{ + mg: fakeSpaceRole(withExternalName(guidRole)), + obs: managed.ExternalObservation{ResourceExists: false}, + err: nil, + }, + service: func() *fake.MockSpaceRole { + m := &fake.MockSpaceRole{} + m.On("Get", guidRole).Return( + fake.SpaceRoleNil, + errors.New("CF-ResourceNotFound: The role was not found"), + ) + return m + }, + }, + "SetExternalNameInvalidFormat": { + args: args{ + mg: fakeSpaceRole(withExternalName("not-a-valid-guid")), + }, + want: want{ + mg: fakeSpaceRole(withExternalName("not-a-valid-guid")), + obs: managed.ExternalObservation{}, + err: errors.New("external-name 'not-a-valid-guid' is not a valid GUID format"), + }, + service: func() *fake.MockSpaceRole { + return &fake.MockSpaceRole{} + }, + }, } for n, tc := range cases { @@ -454,15 +499,124 @@ func TestCreate(t *testing.T) { 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.obs, obs); diff != "" { - t.Errorf("Observe(...): -want, +got:\n%s", diff) + t.Errorf("Create(...): -want, +got:\n%s", diff) + } + }) + } +} + +func TestDelete(t *testing.T) { + type service func() *fake.MockSpaceRole + 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: fakeSpaceRole(withExternalName(guidRole), withID("resource-id")), + }, + want: want{ + mg: fakeSpaceRole(withExternalName(guidRole), withID("resource-id"), withConditions(xpv1.Deleting())), + obs: managed.ExternalDelete{}, + err: nil, + }, + service: func() *fake.MockSpaceRole { + m := &fake.MockSpaceRole{} + m.On("Delete").Return( + "job-guid-123", + nil, + ) + return m + }, + }, + "404NotFound": { + args: args{ + mg: fakeSpaceRole(withExternalName(guidRole), withID("resource-id")), + }, + want: want{ + mg: fakeSpaceRole(withExternalName(guidRole), withID("resource-id"), withConditions(xpv1.Deleting())), + obs: managed.ExternalDelete{}, + err: nil, + }, + service: func() *fake.MockSpaceRole { + m := &fake.MockSpaceRole{} + m.On("Delete").Return( + "", + errors.New("CF-ResourceNotFound: The role was not found"), + ) + return m + }, + }, + "Error": { + args: args{ + mg: fakeSpaceRole(withExternalName(guidRole), withID("resource-id")), + }, + want: want{ + mg: fakeSpaceRole(withExternalName(guidRole), withID("resource-id"), withConditions(xpv1.Deleting())), + obs: managed.ExternalDelete{}, + err: errors.Wrap(errors.New("CF-ResourceNotDeleted: The role could not be deleted"), errDelete), + }, + service: func() *fake.MockSpaceRole { + m := &fake.MockSpaceRole{} + m.On("Delete").Return( + "", + errors.New("CF-ResourceNotDeleted: The role could not be deleted"), + ) + return m + }, + }, + } + for n, tc := range cases { + t.Run(n, func(t *testing.T) { + t.Logf("Testing: %s", t.Name()) + mockJob := &fake.MockJob{} + mockJob.On("PollComplete").Return(nil) + + c := &external{ + kube: &test.MockClient{ + MockDelete: test.NewMockDeleteFn(nil), + }, + job: mockJob, + role: tc.service(), + } + + obs, err := c.Delete(context.Background(), tc.args.mg) + + 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("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/package/crds/cloudfoundry.crossplane.io_spaceroles.yaml b/package/crds/cloudfoundry.crossplane.io_spaceroles.yaml index 27018483..8a6b2211 100644 --- a/package/crds/cloudfoundry.crossplane.io_spaceroles.yaml +++ b/package/crds/cloudfoundry.crossplane.io_spaceroles.yaml @@ -34,9 +34,15 @@ spec: name: v1alpha1 schema: openAPIV3Schema: - description: SpaceRole is the Schema for the OrgRoles API. Provides a Cloud - Foundry resource for assigning org roles.(Updating a role is not supported - according to the docs) + description: |- + SpaceRole is the Schema for the OrgRoles API. Provides a Cloud Foundry resource for assigning org roles.(Updating a role is not supported according to the docs) + + External-Name Configuration: + - Follows Standard: yes + - Format: Space Role GUID (UUID format) + - How to find: + - UI: Not available in the BTP Cockpit + - CLI: Use CF CLI: `cf space-users -v` and find the GUID in the output properties: apiVersion: description: |- diff --git a/test/e2e/cloudfoundry_spacerole_import_test.go b/test/e2e/cloudfoundry_spacerole_import_test.go new file mode 100644 index 00000000..930a95c3 --- /dev/null +++ b/test/e2e/cloudfoundry_spacerole_import_test.go @@ -0,0 +1,44 @@ +//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 ( + spaceRoleImportTestK8sResName = "e2e-test-space-role-import" + spaceRoleImportTestType = "Developer" + spaceRoleImportTestUsername = "user1@example.com" + spaceRoleImportTestOrgName = "cf-ci-e2e" + spaceRoleImportTestSpaceName = "import-test-space-donotdelete" +) + +func TestSpaceRoleImportFlow(t *testing.T) { + importTester := NewImportTester( + &v1alpha1.SpaceRole{ + Spec: v1alpha1.SpaceRoleSpec{ + ForProvider: v1alpha1.SpaceRoleParameters{ + SpaceReference: v1alpha1.SpaceReference{ + SpaceName: &spaceRoleImportTestSpaceName, + OrgName: &spaceRoleImportTestOrgName, + }, + Type: spaceRoleImportTestType, + Username: spaceRoleImportTestUsername, + }, + }, + }, + spaceRoleImportTestK8sResName, + WithWaitCreateTimeout[*v1alpha1.SpaceRole](wait.WithTimeout(5*time.Minute)), + WithWaitDeletionTimeout[*v1alpha1.SpaceRole](wait.WithTimeout(5*time.Minute)), + ) + + importFeature := importTester.BuildTestFeature("CF Space Role Import Flow").Feature() + + testenv.Test(t, importFeature) + +} diff --git a/test/e2e/import_utils.go b/test/e2e/import_utils.go new file mode 100644 index 00000000..94a6a769 --- /dev/null +++ b/test/e2e/import_utils.go @@ -0,0 +1,135 @@ +//go:build e2e + +package e2e + +import ( + "context" + "fmt" + "testing" + + "github.com/SAP/crossplane-provider-cloudfoundry/test" + "github.com/crossplane-contrib/xp-testing/pkg/resources" + xpv1 "github.com/crossplane/crossplane-runtime/apis/common/v1" + "github.com/crossplane/crossplane-runtime/pkg/resource" + v1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/klog/v2" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/e2e-framework/klient/k8s" + wairres "sigs.k8s.io/e2e-framework/klient/k8s/resources" + "sigs.k8s.io/e2e-framework/klient/wait" + "sigs.k8s.io/e2e-framework/klient/wait/conditions" + "sigs.k8s.io/e2e-framework/pkg/envconf" +) + +type mockList struct { + client.ObjectList + + Items []k8s.Object +} + +func waitForResource(res k8s.Object, cfg *envconf.Config, t *testing.T, opts ...wait.Option) { + client := cfg.Client() + + c := conditions.New(client.Resources()) + + match := c.ResourceMatch(res, func(object k8s.Object) bool { + d := object.(resource.Conditioned) + condition := d.GetCondition(xpv1.Available().Type) + result := condition.Status == v1.ConditionTrue + klog.V(4).Infof( + "Checking %s on %v. result=%v", + resources.Identifier(res), + condition, + condition.Status == v1.ConditionTrue, + ) + return result + }) + + err := wait.For(match, opts...) + + if err != nil { + t.Fatal(err) + } +} + +// MustGetResource generic loading of resources, potential errors are passed to the testing context +func MustGetResource[T k8s.Object](t *testing.T, cfg *envconf.Config, name string, ns *string, ct T) T { + res, err := GetResource(cfg, name, ns, ct) + if err != nil { + t.Error("Failed to get resource. error : ", err) + } + return res +} + +// GetResource generic loading of resources from config, returns potential err +func GetResource[T k8s.Object](cfg *envconf.Config, name string, ns *string, ct T) (T, error) { + var namespace string + if ns != nil { + namespace = *ns + } else { + namespace = cfg.Namespace() + } + r := cfg.Client().Resources() + + err := r.Get(context.TODO(), name, namespace, ct) + return ct, err +} + +// DeleteResourcesIgnoreMissing deletes resources defined in a certain directory relative to testdata/crs/ +func DeleteResourcesIgnoreMissing(ctx context.Context, t *testing.T, cfg *envconf.Config, manifestDir string, timeout wait.Option) context.Context { + klog.V(4).Info("Attempt to delete previously imported resources") + r, _ := GetResourcesWithRESTConfig(cfg) + objects, err := test.GetObjectsToImport(ctx, cfg, []string{manifestDir}) + if err != nil { + t.Fatal(err) + } + for _, obj := range objects { + delErr := r.Delete(ctx, obj) + if delErr != nil && !errors.IsNotFound(delErr) { + t.Fatal(delErr) + } + } + + if err = wait.For( + conditions.New(r).ResourcesDeleted(&mockList{Items: objects}), + timeout, + ); err != nil { + t.Fatal(err) + } + return ctx +} + +// AwaitResourceDeletionOrFail deletes a given k8s object with a timeout of configurable duration +// this should be moved into xp-testing library +func AwaitResourceDeletionOrFail(ctx context.Context, t *testing.T, cfg *envconf.Config, object k8s.Object, opts ...wait.Option) { + res := cfg.Client().Resources() + + err := res.Delete(ctx, object) + if err != nil { + t.Fatalf("Failed to delete object %s: %s.", identifier(object), err) + } + + err = wait.For(conditions.New(res).ResourceDeleted(object), opts...) + if err != nil { + t.Fatalf( + "Failed to delete object in time %s.", + identifier(object), + ) + } +} + +// GetResourcesWithRESTConfig returns new resource from REST config +func GetResourcesWithRESTConfig(cfg *envconf.Config) (*wairres.Resources, error) { + r, err := wairres.New(cfg.Client().RESTConfig()) + return r, err +} + +// Identifier returns k8s object name +func identifier(object k8s.Object) string { + return fmt.Sprintf("%s/%s", object.GetObjectKind().GroupVersionKind().String(), object.GetName()) +} + +func NewID(oldId string, buildId string) string { + return buildId + "-" + oldId +} diff --git a/test/e2e/import_utils_test.go b/test/e2e/import_utils_test.go new file mode 100644 index 00000000..546ecbcb --- /dev/null +++ b/test/e2e/import_utils_test.go @@ -0,0 +1,202 @@ +//go:build e2e + +package e2e + +import ( + "context" + "testing" + "time" + + meta "github.com/SAP/crossplane-provider-cloudfoundry/apis" + "github.com/crossplane-contrib/xp-testing/pkg/envvar" + "github.com/crossplane-contrib/xp-testing/pkg/resources" + xpv1 "github.com/crossplane/crossplane-runtime/apis/common/v1" + xpmeta "github.com/crossplane/crossplane-runtime/pkg/meta" + "github.com/crossplane/crossplane-runtime/pkg/resource" + res "sigs.k8s.io/e2e-framework/klient/k8s/resources" + + "k8s.io/klog/v2" + "sigs.k8s.io/e2e-framework/klient/wait" + "sigs.k8s.io/e2e-framework/pkg/envconf" + "sigs.k8s.io/e2e-framework/pkg/features" +) + +var importManagementPolicies = []xpv1.ManagementAction{ + xpv1.ManagementActionObserve, + xpv1.ManagementActionCreate, + xpv1.ManagementActionUpdate, + xpv1.ManagementActionLateInitialize, +} + +var UUT_BUILD_ID_KEY = "BUILD_ID" + +const ( + importFeatureContextKey = "importExternalName" +) + +// ImportTester helps to build e2e test feature for import flow of a managed resource. +// T is the type of the managed resource to be imported. +// Use NewImportTester to create an instance, then use BuildTestFeature to build the test feature. +// Use ImportTesterOption to customize timeouts. +type ImportTester[T resource.Managed] struct { + //will be used as importing resource. The ObjectMeta.Name will be set automatically. + BaseResource T + + // will be prefixed with BUILD_ID to ensure uniqueness + BaseName string + + // the path to the dependent resource yaml files, if any + DependentResourceDirectory string + + // the timeout for waiting till dependent resources get healthy (in setup) + WaitDependentResourceTimeout wait.Option + + // the timeout for waiting till target resource get healthy after creating + WaitCreateTimeout wait.Option + + // the timeout for waiting till resource get deleted (in setup and teardown) + WaitDeletionTimeout wait.Option +} + +type ImportTesterOption[T resource.Managed] func(*ImportTester[T]) + +func WithWaitDependentResourceTimeout[T resource.Managed](timeout wait.Option) ImportTesterOption[T] { + return func(it *ImportTester[T]) { + it.WaitDependentResourceTimeout = timeout + } +} + +func WithWaitCreateTimeout[T resource.Managed](timeout wait.Option) ImportTesterOption[T] { + return func(it *ImportTester[T]) { + it.WaitCreateTimeout = timeout + } +} + +func WithWaitDeletionTimeout[T resource.Managed](timeout wait.Option) ImportTesterOption[T] { + return func(it *ImportTester[T]) { + it.WaitDeletionTimeout = timeout + } +} + +func WithDependentResourceDirectory[T resource.Managed](path string) ImportTesterOption[T] { + return func(it *ImportTester[T]) { + it.DependentResourceDirectory = path + } +} + +// NewImportTester creates an ImportTester for the given managed resource and base name. +// The base name will be prefixed with BUILD_ID to ensure uniqueness. +// Additional options can be provided to customize timeouts using ImportTesterOption. +func NewImportTester[T resource.Managed](baseResource T, baseName string, o ...ImportTesterOption[T]) *ImportTester[T] { + it := &ImportTester[T]{ + BaseResource: baseResource, + BaseName: baseName, + WaitDependentResourceTimeout: wait.WithTimeout(5 * time.Minute), + WaitCreateTimeout: wait.WithTimeout(3 * time.Minute), + WaitDeletionTimeout: wait.WithTimeout(3 * time.Minute), + } + it.BaseResource.SetName(it.GetPrefixedName()) + + for _, opt := range o { + opt(it) + } + + return it +} + +func (it *ImportTester[T]) GetPrefixedName() string { + return NewID(it.BaseName, envvar.GetOrDefault(UUT_BUILD_ID_KEY, "0000")) +} + +func (it *ImportTester[T]) BuildTestFeature(name string) *features.FeatureBuilder { + return features.New(name). + Setup( + func(ctx context.Context, t *testing.T, cfg *envconf.Config) context.Context { + r, _ := res.New(cfg.Client().RESTConfig()) + _ = meta.AddToScheme(r.GetScheme()) + + if it.DependentResourceDirectory != "" { + log("Applying dependent resources from "+it.DependentResourceDirectory, it.BaseResource, func() { + resources.ImportResources(ctx, t, cfg, it.DependentResourceDirectory) + + if err := resources.WaitForResourcesToBeSynced(ctx, cfg, it.DependentResourceDirectory, nil, it.WaitDependentResourceTimeout); err != nil { + resources.DumpManagedResources(ctx, t, cfg) + t.Fatal(err) + } + }) + } + + //prepare the resource for creation + createResource := it.BaseResource.DeepCopyObject().(T) + createResource.SetManagementPolicies(importManagementPolicies) + + log("Creating resource on external system to be imported later", createResource, func() { + if err := cfg.Client().Resources().Create(ctx, createResource); err != nil { + t.Fatalf("Failed to create resource for import test: %v", err) + } + waitForResource(createResource, cfg, t, it.WaitCreateTimeout) + }) + + createdResource := it.BaseResource.DeepCopyObject().(T) + log("Getting created resource to obtain external name", createResource, func() { + MustGetResource(t, cfg, it.GetPrefixedName(), nil, createdResource) + externalName := xpmeta.GetExternalName(createdResource) + ctx = context.WithValue(ctx, importFeatureContextKey, externalName) + }) + + // delete the created resource to prepare for import. With managment policies missing Delete, it will not be deleted in the external system + log("Deleting resource", createdResource, func() { + AwaitResourceDeletionOrFail(ctx, t, cfg, createdResource, it.WaitDeletionTimeout) + }) + + return ctx + }, + ).Assess( + "Check Imported Resource gets healthy", func(ctx context.Context, t *testing.T, cfg *envconf.Config) context.Context { + externalName := ctx.Value(importFeatureContextKey).(string) + + //preare the resource for import + resource := it.BaseResource.DeepCopyObject().(T) + xpmeta.SetExternalName(resource, externalName) + resource.SetManagementPolicies(xpv1.ManagementPolicies{xpv1.ManagementActionAll}) + + //create the resource again for importing, should match the external resource + log("Create MR for importing", resource, func() { + if err := cfg.Client().Resources().Create(ctx, resource); err != nil { + t.Fatalf("Failed to create cr when importing: %v", err) + } + }) + + log("Waiting for imported resource to become healthy", resource, func() { + waitForResource(resource, cfg, t, it.WaitCreateTimeout) + }) + return ctx + }, + ).Teardown( + func(ctx context.Context, t *testing.T, cfg *envconf.Config) context.Context { + resource := it.BaseResource.DeepCopyObject().(T) + MustGetResource(t, cfg, it.GetPrefixedName(), nil, resource) + + log("Deleting imported resource", resource, func() { + AwaitResourceDeletionOrFail(ctx, t, cfg, resource, it.WaitDeletionTimeout) + }) + + log("Deleting dependent resources", resource, func() { + if it.DependentResourceDirectory != "" { + DeleteResourcesIgnoreMissing(ctx, t, cfg, it.DependentResourceDirectory, it.WaitDeletionTimeout) + } + }) + return ctx + }, + ) +} + +// log is a helper function to log the start and end of an operation on a managed resource with name and external name. +func log(msg string, mr resource.Managed, f func(), keysAndValues ...any) { + kAndV := []interface{}{"name", mr.GetName(), "external-name", xpmeta.GetExternalName(mr)} + kAndV = append(kAndV, keysAndValues...) + + klog.InfoS("STARTING: "+msg, kAndV...) + f() + klog.InfoS("DONE: "+msg, kAndV...) +} diff --git a/test/upgrade/space_external_name_upgrade_test.go b/test/upgrade/space_external_name_upgrade_test.go index 971445ef..4005aeba 100644 --- a/test/upgrade/space_external_name_upgrade_test.go +++ b/test/upgrade/space_external_name_upgrade_test.go @@ -25,7 +25,8 @@ import ( var ( customResourceDirectories = []string{ - "./testdata/customCrs/externalNames", + "./testdata/customCrs/externalNames/import", + "./testdata/customCrs/externalNames/space", } ) diff --git a/test/upgrade/space_role_external_name_upgrade_test.go b/test/upgrade/space_role_external_name_upgrade_test.go new file mode 100644 index 00000000..0c1c33c4 --- /dev/null +++ b/test/upgrade/space_role_external_name_upgrade_test.go @@ -0,0 +1,114 @@ +//go:build upgrade + +// +// This file (space_external_name_upgrade_test.go) contains Test_Space_External_Name, +// which validates that Space 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 +// +// This test demonstrates the use of CustomUpgradeTestBuilder for creating +// specialized upgrade tests with custom pre/post-upgrade validation logic. + +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 ( + spaceRoleCustomResourceDirectories = []string{ + "./testdata/customCrs/externalNames/import", + "./testdata/customCrs/externalNames/spaceRole", + } +) + +func Test_Space_Role_External_Name(t *testing.T) { + const spaceRoleName = "upgrade-test-external-name-space-role" + + upgradeTest := NewCustomUpgradeTest("space-external-name-test"). + FromVersion(fromTag). + ToVersion(toTag). + WithResourceDirectories(spaceRoleCustomResourceDirectories). + 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) + } + + spaceRole := &v1alpha1.SpaceRole{} + + err = r.Get(ctx, spaceRoleName, cfg.Namespace(), spaceRole) + if err != nil { + t.Fatalf("Failed to get SpaceRole resource: %v", err) + } + + annotations := spaceRole.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, "preUpgradeExternalName", externalName) + }, + ). + WithCustomPostUpgradeAssessment( + "Verify external name after upgrade", + func(ctx context.Context, t *testing.T, cfg *envconf.Config) context.Context { + spaceRole := &v1alpha1.SpaceRole{} + r := cfg.Client().Resources() + + err := r.Get(ctx, spaceRoleName, cfg.Namespace(), spaceRole) + if err != nil { + t.Fatalf("Failed to get SpaceRole resource: %v", err) + } + + annotations := spaceRole.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("preUpgradeExternalName").(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/import.yaml b/test/upgrade/testdata/customCrs/externalNames/import.yaml deleted file mode 100644 index 15f90087..00000000 --- a/test/upgrade/testdata/customCrs/externalNames/import.yaml +++ /dev/null @@ -1,11 +0,0 @@ -apiVersion: cloudfoundry.crossplane.io/v1alpha1 -kind: Organization -metadata: - name: upgrade-test-org -spec: - managementPolicies: - - Observe - forProvider: - # IMPORTANT: Change this to an org you have access to - # Run `cf orgs` to see available organizations - name: cf-ci-e2e # ← TODO: Change this to an existing org name \ No newline at end of file diff --git a/test/upgrade/testdata/customCrs/externalNames/import/import.yaml b/test/upgrade/testdata/customCrs/externalNames/import/import.yaml new file mode 100644 index 00000000..6b1829e9 --- /dev/null +++ b/test/upgrade/testdata/customCrs/externalNames/import/import.yaml @@ -0,0 +1,27 @@ +apiVersion: cloudfoundry.crossplane.io/v1alpha1 +kind: Organization +metadata: + name: upgrade-test-org +spec: + managementPolicies: + - Observe + forProvider: + # IMPORTANT: Change this to an org you have access to + # Run `cf orgs` to see available organizations + name: cf-ci-e2e # ← TODO: Change this to an existing org name +--- +apiVersion: cloudfoundry.crossplane.io/v1alpha1 +kind: Space +metadata: + name: upgrade-test-import-space + annotations: + crossplane.io/paused: "false" +spec: + managementPolicies: + - Observe + forProvider: + name: upgrade-test-space-donotdelete + orgRef: + name: upgrade-test-org # ← References the imported org + providerConfigRef: + name: default \ No newline at end of file diff --git a/test/upgrade/testdata/customCrs/externalNames/space.yaml b/test/upgrade/testdata/customCrs/externalNames/space/space.yaml similarity index 100% rename from test/upgrade/testdata/customCrs/externalNames/space.yaml rename to test/upgrade/testdata/customCrs/externalNames/space/space.yaml diff --git a/test/upgrade/testdata/customCrs/externalNames/spaceRole/space_role.yaml b/test/upgrade/testdata/customCrs/externalNames/spaceRole/space_role.yaml new file mode 100644 index 00000000..0b6fea1a --- /dev/null +++ b/test/upgrade/testdata/customCrs/externalNames/spaceRole/space_role.yaml @@ -0,0 +1,17 @@ +apiVersion: cloudfoundry.crossplane.io/v1alpha1 +kind: SpaceRole +metadata: + name: upgrade-test-external-name-space-role + annotations: + crossplane.io/paused: "false" +spec: + forProvider: + type: Developer + username: user1@example.com + spaceRef: + name: upgrade-test-import-space + policy: + resolution: Required + resolve: Always + providerConfigRef: + name: default \ No newline at end of file