SYSENG-1822: panic when faking ResourceWithTag

Reverts commit e937d73. The source of the issue was faking ResourceWithTag instead of setting tags or tagging a resource.

pkg/api/mock/mock_api_implementation.go

@@ -296,6 +296,11 @@ func (a *mockAPI) FakeExisting(o types.Object, tags ...string) string {
 	a.dataMu.Lock()
 	defer a.dataMu.Unlock()
 
+	// SYSENG-1822: faking ResourceWithTag blindly overwrites object with same identifier.
+	if _, ok := o.(*corev1.ResourceWithTag); ok {
+		panic("Cannot fake ResourceWithTag, pass tags to the object instead.")
+	}
+
 	identifier := makeObjectIdentifiable(o)
 
 	mao := APIObject{

pkg/api/mock/mock_api_implementation_test.go

@@ -175,13 +175,13 @@ var _ = Describe("Mock API implementation", func() {
 
 		It("supports corev1.ResourceWithTag", func() {
 			id := a.FakeExisting(&testObject{})
-			err := a.Create(context.TODO(), &corev1.ResourceWithTag{ResourceIdentifier: id, Tag: "tagname"})
+			err := a.Create(context.TODO(), &corev1.ResourceWithTag{Identifier: id, Tag: "tagname"})
 			Expect(err).ToNot(HaveOccurred())
 			Expect(a.Inspect(id).HasTags("tagname")).To(BeTrue())
 		})
 
 		It("handles errors on tag operation", func() {
-			err := a.Create(context.TODO(), &corev1.ResourceWithTag{ResourceIdentifier: "not-in-api", Tag: "tagname"})
+			err := a.Create(context.TODO(), &corev1.ResourceWithTag{Identifier: "not-in-api", Tag: "tagname"})
 			Expect(err).To(MatchError(api.ErrNotFound))
 		})
 	})
@@ -237,14 +237,14 @@ var _ = Describe("Mock API implementation", func() {
 
 		It("supports corev1.ResourceWithTag", func() {
 			id := a.FakeExisting(&testObject{}, "tagname")
-			err := a.Destroy(context.TODO(), &corev1.ResourceWithTag{ResourceIdentifier: id, Tag: "tagname"})
+			err := a.Destroy(context.TODO(), &corev1.ResourceWithTag{Identifier: id, Tag: "tagname"})
 			Expect(err).ToNot(HaveOccurred())
 			Expect(a.Inspect(id).HasTags("tagname")).To(BeFalse())
 		})
 
 		It("returns 404 if object to be untagged does not have tag provided in corev1.ResourceWithTag", func() {
 			id := a.FakeExisting(&testObject{})
-			err := a.Destroy(context.TODO(), &corev1.ResourceWithTag{ResourceIdentifier: id, Tag: "tagname"})
+			err := a.Destroy(context.TODO(), &corev1.ResourceWithTag{Identifier: id, Tag: "tagname"})
 			Expect(err).To(MatchError(api.ErrNotFound))
 		})
 

pkg/api/mock/mock_api_tags.go

@@ -25,7 +25,7 @@ func (a *mockAPI) tagOperation(o types.Object, op tagOperation) (bool, error) {
 		return false, nil
 	}
 
-	obj, ok := a.data[rwt.ResourceIdentifier]
+	obj, ok := a.data[rwt.Identifier]
 	if !ok {
 		return true, api.ErrNotFound
 	}

pkg/api/mock/mock_api_tags_test.go

@@ -29,7 +29,7 @@ var _ = Describe("Mock API Tags", func() {
 	})
 
 	It("returns api.ErrNotFound when object to tag does not exist in mock API", func() {
-		isTagOperation, err := a.tagOperation(&corev1.ResourceWithTag{ResourceIdentifier: "not-in-api"}, tagOperationCreate)
+		isTagOperation, err := a.tagOperation(&corev1.ResourceWithTag{Identifier: "not-in-api"}, tagOperationCreate)
 		Expect(isTagOperation).To(BeTrue())
 		Expect(err).To(MatchError(api.ErrNotFound))
 	})
@@ -41,7 +41,7 @@ var _ = Describe("Mock API Tags", func() {
 	})
 
 	It("returns api.HTTPError (422) when an object is tagged more than once with the same tag", func() {
-		rwt := &corev1.ResourceWithTag{ResourceIdentifier: objects[0].Identifier, Tag: "tagname"}
+		rwt := &corev1.ResourceWithTag{Identifier: objects[0].Identifier, Tag: "tagname"}
 
 		isTagOperation, err := a.tagOperation(rwt, tagOperationCreate)
 		Expect(isTagOperation).To(BeTrue())
@@ -55,15 +55,15 @@ var _ = Describe("Mock API Tags", func() {
 	})
 
 	It("returns api.HTTPError (404) when an object is untagged from a tag it doesn't have", func() {
-		rwt := &corev1.ResourceWithTag{ResourceIdentifier: objects[0].Identifier, Tag: "tagname"}
+		rwt := &corev1.ResourceWithTag{Identifier: objects[0].Identifier, Tag: "tagname"}
 
 		isTagOperation, err := a.tagOperation(rwt, tagOperationDestroy)
 		Expect(isTagOperation).To(BeTrue())
 		Expect(err).To(MatchError(api.ErrNotFound))
 	})
 
 	It("returns errTagOperationNotSupported if an unsupported tag operation was provided", func() {
-		rwt := &corev1.ResourceWithTag{ResourceIdentifier: objects[0].Identifier, Tag: "tagname"}
+		rwt := &corev1.ResourceWithTag{Identifier: objects[0].Identifier, Tag: "tagname"}
 
 		isTagOperation, err := a.tagOperation(rwt, 3)
 		Expect(isTagOperation).To(BeTrue())

pkg/apis/core/v1/resource_genclient.go

@@ -91,10 +91,6 @@ func (rwt ResourceWithTag) EndpointURL(ctx context.Context) (*url.URL, error) {
 		return nil, fmt.Errorf("%w: ResourceWithTag only support Create and Destroy operations", api.ErrOperationNotSupported)
 	}
 
-	if rwt.ResourceIdentifier != "" {
-		return url.Parse(fmt.Sprintf("/api/core/v1/resource.json/%v/tags/%v", rwt.ResourceIdentifier, rwt.Tag))
-	}
-	// SYSENG-1822: keep backwards compatibility when only providing 'Identifier'.
 	return url.Parse(fmt.Sprintf("/api/core/v1/resource.json/%v/tags/%v", rwt.Identifier, rwt.Tag))
 }
 
@@ -108,8 +104,7 @@ func (rwt ResourceWithTag) FilterRequestURL(ctx context.Context, url *url.URL) (
 		return nil, fmt.Errorf("%w: ResourceWithTag only support Create and Destroy operations", api.ErrOperationNotSupported)
 	}
 
-	// remove 'Identifier' from path added by API client
-	if op == types.OperationDestroy && rwt.Identifier != "" {
+	if op == types.OperationDestroy {
 		url.Path = path.Dir(url.Path)
 	}
 

pkg/apis/core/v1/resource_tags.go

@@ -69,7 +69,7 @@ func resourceWithTagObjects(obj types.IdentifiedObject, tags ...string) ([]*Reso
 	objects := make([]*ResourceWithTag, 0, len(tags))
 
 	for _, tag := range tags {
-		objects = append(objects, &ResourceWithTag{ResourceIdentifier: identifier, Tag: tag})
+		objects = append(objects, &ResourceWithTag{Identifier: identifier, Tag: tag})
 	}
 
 	return objects, nil

pkg/apis/core/v1/resource_test.go

@@ -18,7 +18,7 @@ import (
 
 var _ = Describe("resource.Resource", func() {
 	Context("ResourceWithTags", func() {
-		rwt := &corev1.ResourceWithTag{ResourceIdentifier: "test-identifier", Tag: "test-tag"}
+		rwt := &corev1.ResourceWithTag{Identifier: "test-identifier", Tag: "test-tag"}
 
 		DescribeTable("Test EndpointURL and FilterRequestURL for all operations", func(op types.Operation, errorMatcher gomegaTypes.GomegaMatcher, expectedPath string) {
 			singleObjectOperation := op == types.OperationGet || op == types.OperationUpdate || op == types.OperationDestroy
@@ -47,37 +47,6 @@ var _ = Describe("resource.Resource", func() {
 			Entry("When operation is List", types.OperationList, MatchError(api.ErrOperationNotSupported), ""),
 			Entry("When operation is Update", types.OperationUpdate, MatchError(api.ErrOperationNotSupported), ""),
 		)
-
-		// cover former usage with 'Identifier'
-		rwt2 := &corev1.ResourceWithTag{Identifier: "test-identifier", Tag: "test-tag"}
-
-		DescribeTable("Test EndpointURL and FilterRequestURL for all operations", func(op types.Operation, errorMatcher gomegaTypes.GomegaMatcher, expectedPath string) {
-			singleObjectOperation := op == types.OperationGet || op == types.OperationUpdate || op == types.OperationDestroy
-			ctxWithOperation := types.ContextWithOperation(
-				context.TODO(),
-				op,
-			)
-
-			url, err := rwt2.EndpointURL(ctxWithOperation)
-			Expect(err).To(errorMatcher)
-
-			if err == nil {
-				Expect(url.Path).To(BeEquivalentTo(expectedPath))
-				// API client appends objects identifier to path on singleObjectOperation which should be removed by FilterRequestURLHook
-				if singleObjectOperation {
-					url.Path = path.Join(url.Path, rwt2.Identifier)
-				}
-				filteredURL, err := rwt2.FilterRequestURL(ctxWithOperation, url)
-				Expect(err).To(errorMatcher)
-				Expect(filteredURL.Path).To(BeEquivalentTo(expectedPath))
-			}
-		},
-			Entry("When operation is Create", types.OperationCreate, BeNil(), "/api/core/v1/resource.json/test-identifier/tags/test-tag"),
-			Entry("When operation is Destroy", types.OperationDestroy, BeNil(), "/api/core/v1/resource.json/test-identifier/tags/test-tag"),
-			Entry("When operation is Get", types.OperationGet, MatchError(api.ErrOperationNotSupported), ""),
-			Entry("When operation is List", types.OperationList, MatchError(api.ErrOperationNotSupported), ""),
-			Entry("When operation is Update", types.OperationUpdate, MatchError(api.ErrOperationNotSupported), ""),
-		)
 	})
 
 	Context("RetryResourceTagging", func() {

pkg/apis/core/v1/resource_types.go

@@ -25,11 +25,8 @@ type Resource struct {
 
 // ResourceWithTag is a virtual Object used to add (Create) or remove (Destroy) a tag to/from a Resource.
 type ResourceWithTag struct {
-	// Identifier of the tag that will be created
-	Identifier string `anxcloud:"identifier"`
-
 	// Identifier of the Resource which tags to change
-	ResourceIdentifier string `json:"resource_identifier"`
+	Identifier string `anxcloud:"identifier"`
 
 	// Name of the Tag to add or remove from the resource
 	Tag string