Implement feedback

pkg/api/api_implementation.go

@@ -92,8 +92,6 @@ func (a defaultAPI) Get(ctx context.Context, o types.IdentifiedObject, opts ...t
 	for _, requestOpt := range a.requestOptions {
 		if getOpt, ok := requestOpt.(types.GetOption); ok {
 			err = errors.Join(err, getOpt.ApplyToGet(&options))
-		} else if anyOpt, ok := requestOpt.(types.AnyOption); ok {
-			err = errors.Join(err, anyOpt.ApplyToAny(&options))
 		}
 	}
 	for _, opt := range opts {
@@ -113,8 +111,6 @@ func (a defaultAPI) Create(ctx context.Context, o types.Object, opts ...types.Cr
 	for _, requestOpt := range a.requestOptions {
 		if createOpt, ok := requestOpt.(types.CreateOption); ok {
 			err = errors.Join(err, createOpt.ApplyToCreate(&options))
-		} else if anyOpt, ok := requestOpt.(types.AnyOption); ok {
-			err = errors.Join(err, anyOpt.ApplyToAny(&options))
 		}
 	}
 	for _, opt := range opts {
@@ -150,8 +146,6 @@ func (a defaultAPI) Update(ctx context.Context, o types.IdentifiedObject, opts .
 	for _, requestOpt := range a.requestOptions {
 		if updateOpt, ok := requestOpt.(types.UpdateOption); ok {
 			err = errors.Join(err, updateOpt.ApplyToUpdate(&options))
-		} else if anyOpt, ok := requestOpt.(types.AnyOption); ok {
-			err = errors.Join(err, anyOpt.ApplyToAny(&options))
 		}
 	}
 	for _, opt := range opts {
@@ -171,8 +165,6 @@ func (a defaultAPI) Destroy(ctx context.Context, o types.IdentifiedObject, opts
 	for _, requestOpt := range a.requestOptions {
 		if destroyOpt, ok := requestOpt.(types.DestroyOption); ok {
 			err = errors.Join(err, destroyOpt.ApplyToDestroy(&options))
-		} else if anyOpt, ok := requestOpt.(types.AnyOption); ok {
-			err = errors.Join(err, anyOpt.ApplyToAny(&options))
 		}
 	}
 	for _, opt := range opts {
@@ -192,8 +184,6 @@ func (a defaultAPI) List(ctx context.Context, o types.FilterObject, opts ...type
 	for _, requestOpt := range a.requestOptions {
 		if listOpt, ok := requestOpt.(types.ListOption); ok {
 			err = errors.Join(err, listOpt.ApplyToList(&options))
-		} else if anyOpt, ok := requestOpt.(types.AnyOption); ok {
-			err = errors.Join(err, anyOpt.ApplyToAny(&options))
 		}
 	}
 	for _, opt := range opts {

pkg/api/api_test.go

@@ -94,36 +94,10 @@ func (o apiTestAnyopOption) ApplyToDestroy(opts *types.DestroyOptions) error {
 	return opts.Set("api_test_option", o, false)
 }
 
-type errorOption struct {
-	err error
-}
-
-func (o errorOption) ApplyToGet(opts *types.GetOptions) error {
-	return o.err
-}
-
-func (o errorOption) ApplyToList(opts *types.ListOptions) error {
-	return o.err
-}
-
-func (o errorOption) ApplyToCreate(opts *types.CreateOptions) error {
-	return o.err
-}
-
-func (o errorOption) ApplyToUpdate(opts *types.UpdateOptions) error {
-	return o.err
-}
-
-func (o errorOption) ApplyToDestroy(opts *types.DestroyOptions) error {
-	return o.err
-}
-
-type errorAnyOption struct {
-	err error
-}
-
-func (o errorAnyOption) ApplyToAny(opts types.Options) error {
-	return o.err
+func errorOption(err error) types.AnyOption {
+	return func(o types.Options) error {
+		return err
+	}
 }
 
 type apiTestObject struct {
@@ -1111,11 +1085,11 @@ var _ = Describe("using an API object", func() {
 
 		o := apiTestObject{"foo"}
 
-		Expect(api.Create(context.TODO(), &o, errorOption{mockErr})).Error().To(MatchError(mockErr))
-		Expect(api.Get(context.TODO(), &o, errorOption{mockErr})).Error().To(MatchError(mockErr))
-		Expect(api.List(context.TODO(), &o, errorOption{mockErr})).Error().To(MatchError(mockErr))
-		Expect(api.Update(context.TODO(), &o, errorOption{mockErr})).Error().To(MatchError(mockErr))
-		Expect(api.Destroy(context.TODO(), &o, errorOption{mockErr})).Error().To(MatchError(mockErr))
+		Expect(api.Create(context.TODO(), &o, errorOption(mockErr))).Error().To(MatchError(mockErr))
+		Expect(api.Get(context.TODO(), &o, errorOption(mockErr))).Error().To(MatchError(mockErr))
+		Expect(api.List(context.TODO(), &o, errorOption(mockErr))).Error().To(MatchError(mockErr))
+		Expect(api.Update(context.TODO(), &o, errorOption(mockErr))).Error().To(MatchError(mockErr))
+		Expect(api.Destroy(context.TODO(), &o, errorOption(mockErr))).Error().To(MatchError(mockErr))
 	})
 
 	It("returns an error when applying configured default options return errors", func() {
@@ -1126,28 +1100,7 @@ var _ = Describe("using an API object", func() {
 				client.BaseURL(server.URL()),
 				client.IgnoreMissingToken(),
 			),
-			WithRequestOptions(errorOption{mockErr}),
-		)
-		Expect(err).NotTo(HaveOccurred())
-
-		o := apiTestObject{"foo"}
-
-		Expect(api.Create(context.TODO(), &o)).Error().To(MatchError(mockErr))
-		Expect(api.Get(context.TODO(), &o)).Error().To(MatchError(mockErr))
-		Expect(api.List(context.TODO(), &o)).Error().To(MatchError(mockErr))
-		Expect(api.Update(context.TODO(), &o)).Error().To(MatchError(mockErr))
-		Expect(api.Destroy(context.TODO(), &o)).Error().To(MatchError(mockErr))
-	})
-
-	It("returns an error when applying configured default AnyOption return errors", func() {
-		mockErr := errors.New("foo")
-		api, err := NewAPI(
-			WithLogger(logger),
-			WithClientOptions(
-				client.BaseURL(server.URL()),
-				client.IgnoreMissingToken(),
-			),
-			WithRequestOptions(errorAnyOption{mockErr}),
+			WithRequestOptions(errorOption(mockErr)),
 		)
 		Expect(err).NotTo(HaveOccurred())
 

pkg/api/options.go

@@ -2,7 +2,6 @@ package api
 
 import (
 	"context"
-	"errors"
 	"fmt"
 
 	"go.anx.io/go-anxcloud/pkg/api/internal"
@@ -41,29 +40,20 @@ func AutoTag(tags ...string) CreateOption {
 
 // EnvironmentOption can be used to configure an alternative environment path
 // segment for a given API group
-type EnvironmentOption struct {
-	APIGroup       string
-	EnvPathSegment string
-	Override       bool
-}
-
-// ApplyToAny applies the EnvironmentOption to any request type
-func (o EnvironmentOption) ApplyToAny(opts types.Options) error {
-	return opts.Set(fmt.Sprintf("environment/%s", o.APIGroup), o.EnvPathSegment, o.Override)
+func EnvironmentOption(apiGroup, envPathSegment string, override bool) types.AnyOption {
+	return func(o types.Options) error {
+		return o.SetEnvironment(fmt.Sprintf("environment/%s", apiGroup), envPathSegment, override)
+	}
 }
 
 // GetEnvironmentPathSegment retrieves the environment path segment of a given API group
 // or the provided defaultValue if no environment override is set
-func GetEnvironmentPathSegment(ctx context.Context, apiGroup, defaultValue string) (string, error) {
+func GetEnvironmentPathSegment(ctx context.Context, apiGroup, defaultValue string) string {
 	if options, err := types.OptionsFromContext(ctx); err != nil {
-		return "", err
-	} else if env, err := options.Get(fmt.Sprintf("environment/%s", apiGroup)); errors.Is(err, types.ErrKeyNotSet) {
-		return defaultValue, nil
-	} else if err != nil {
-		return "", err
-	} else if envString, ok := env.(string); !ok {
-		return "", ErrEnvironmentSegmentNotTypeString
+		return defaultValue
+	} else if env, err := options.GetEnvironment(fmt.Sprintf("environment/%s", apiGroup)); err != nil {
+		return defaultValue
 	} else {
-		return envString, nil
+		return env
 	}
 }

pkg/api/types/options.go

@@ -11,14 +11,17 @@ var (
 )
 
 type commonOptions struct {
-	additional map[string]interface{}
+	additional   map[string]interface{}
+	environments map[string]string
 }
 
 // Options is the interface all operation-specific options implement, making it possible to pass all the specific options to the same functions.
 // Specific APIs can use this interface to set additional options, keys should be prefixed with the name of the API. This might be enforced in the future.
 type Options interface {
 	Get(key string) (interface{}, error)
 	Set(key string, value interface{}, overwrite bool) error
+	GetEnvironment(key string) (string, error)
+	SetEnvironment(key, value string, overwrite bool) error
 }
 
 // GetOption is the interface options have to implement to be usable with Get operation.
@@ -51,10 +54,35 @@ type DestroyOption interface {
 	ApplyToDestroy(*DestroyOptions) error
 }
 
-type AnyOption interface {
-	ApplyToAny(Options) error
+// AnyOption is the interface options have to implement to be usable with any operation.
+type AnyOption func(Options) error
+
+// ApplyToGet applies the AnyOption to the GetOptions
+func (ao AnyOption) ApplyToGet(opts *GetOptions) error {
+	return ao(opts)
+}
+
+// ApplyToList applies the AnyOption to the ListOptions
+func (ao AnyOption) ApplyToList(opts *ListOptions) error {
+	return ao(opts)
+}
+
+// ApplyToCreate applies the AnyOption to the CreateOptions
+func (ao AnyOption) ApplyToCreate(opts *CreateOptions) error {
+	return ao(opts)
+}
+
+// ApplyToUpdate applies the AnyOption to the UpdateOptions
+func (ao AnyOption) ApplyToUpdate(opts *UpdateOptions) error {
+	return ao(opts)
+}
+
+// ApplyToDestroy applies the AnyOption to the DestroyOptions
+func (ao AnyOption) ApplyToDestroy(opts *DestroyOptions) error {
+	return ao(opts)
 }
 
+// Option is a dummy interface used for any type of request Options
 type Option interface{}
 
 func (o commonOptions) Get(key string) (interface{}, error) {
@@ -79,3 +107,26 @@ func (o *commonOptions) Set(key string, val interface{}, overwrite bool) error {
 	o.additional[key] = val
 	return nil
 }
+
+func (o commonOptions) GetEnvironment(key string) (string, error) {
+	if o.environments != nil {
+		if v, ok := o.environments[key]; ok {
+			return v, nil
+		}
+	}
+
+	return "", ErrKeyNotSet
+}
+
+func (o *commonOptions) SetEnvironment(key, val string, overwrite bool) error {
+	if o.environments == nil {
+		o.environments = make(map[string]string, 1)
+	}
+
+	if _, alreadySet := o.environments[key]; alreadySet && !overwrite {
+		return ErrKeyAlreadySet
+	}
+
+	o.environments[key] = val
+	return nil
+}

pkg/apis/kubernetes/v1/common.go

@@ -20,10 +20,7 @@ func endpointURL(ctx context.Context, o types.Object, resourcePathName string) (
 		return nil, api.ErrOperationNotSupported
 	}
 
-	env, err := api.GetEnvironmentPathSegment(ctx, "kubernetes/v1", "kubernetes")
-	if err != nil {
-		return nil, fmt.Errorf("get environment: %w", err)
-	}
+	env := api.GetEnvironmentPathSegment(ctx, "kubernetes/v1", "kubernetes")
 
 	// we can ignore the error since the URL is hard-coded known as valid
 	u, _ := url.Parse(fmt.Sprintf("/api/%s/v1/%s.json", env, resourcePathName))

pkg/apis/kubernetes/v1/crud_test.go

@@ -24,11 +24,8 @@ const (
 
 var mockStateOK = map[string]interface{}{"type": gs.StateTypeOK}
 
-func EnvironmentTest() types.Option {
-	return api.EnvironmentOption{
-		APIGroup:       "kubernetes/v1",
-		EnvPathSegment: "kubernetes-test",
-	}
+func EnvironmentTest() types.AnyOption {
+	return api.EnvironmentOption("kubernetes/v1", "kubernetes-test", true)
 }
 
 var _ = Describe("CRUD", Ordered, func() {

pkg/apis/kubernetes/v1/node_pool_test.go

@@ -12,8 +12,7 @@ import (
 var _ = Describe("NodePool resource", func() {
 	It("can filter by cluster", func() {
 		np := &NodePool{Cluster: Cluster{Identifier: "foo"}}
-		url, err := np.EndpointURL(
-			types.ContextWithOperation(types.ContextWithOptions(context.TODO(), &types.ListOptions{}), types.OperationList))
+		url, err := np.EndpointURL(types.ContextWithOperation(context.TODO(), types.OperationList))
 		Expect(err).ToNot(HaveOccurred())
 		Expect(url.Query().Encode()).To(Equal("filters=cluster%3Dfoo"))
 	})