issue-522,implementation for OpenSearch Egress Rules lifecycle on APIv2 #527

OleksiienkoMykyta · 2023-08-21T15:37:34Z

…

worryg0d · 2023-08-30T10:31:03Z

Makefile

@@ -98,7 +98,7 @@ run: manifests generate fmt vet ## Run a controller from your host.
 	go run ./main.go

 .PHONY: docker-build
-docker-build: manifests generate test ## Build docker image with the manager.
+docker-build: manifests generate ## test ## Build docker image with the manager.


Please uncomment the test instruction

worryg0d · 2023-08-30T10:41:40Z

apis/clusterresources/v1beta1/opensearchegressrules_webhook.go

+	opensearchegressruleslog.Info("validate create", "name", r.Name)
+
+	if r.Spec.ClusterID == "" || r.Spec.OpenSearchBindingID == "" || r.Spec.Source == "" {
+		return fmt.Errorf("spec.ClusterID, spec.OpenSearchBindingId, spec.Source must be filled")


In your yaml manifest fields are written in camelCase.

Suggested change

return fmt.Errorf("spec.ClusterID, spec.OpenSearchBindingId, spec.Source must be filled")

return fmt.Errorf("spec.ClusterId, spec.ppenSearchBindingId, spec.source must be filled")

worryg0d · 2023-08-30T10:56:46Z

apis/clusterresources/v1beta1/opensearchegressrules_types.go

+	Name                string `json:"name"`
+	OpenSearchBindingID string `json:"openSearchBindingId"`
+	Source              string `json:"source"`
+	Type                string `json:"type"`


API doc says that Type is not required. Please add ,omitempty tag.

worryg0d · 2023-08-30T10:57:19Z

apis/clusterresources/v1beta1/opensearchegressrules_types.go

+func (er *OpenSearchEgressRules) GetJobID(jobName string) string {
+	return client.ObjectKeyFromObject(er).String() + "/" + jobName
+}


Please remove the unused func

worryg0d · 2023-08-30T11:11:56Z

pkg/instaclustr/config.go

+	OpenSearchEgressRulesEndpoint          = "/cluster-management/v2/resources/applications/opensearch/egress-rules/v2"
+	OpenSearchEgressRuleDeleteEndpoint = "/cluster-management/v2/resources/applications/opensearch/egress-rules/v2/"


Both constants are identical. Please delete the second one.

worryg0d · 2023-08-30T11:13:25Z

controllers/clusterresources/suite_test.go

-var (
-	cfg         *rest.Config
-	k8sClient   client.Client
-	testEnv     *envtest.Environment
-	ctx         context.Context
-	cancel      context.CancelFunc
-	MockInstAPI = mock.NewInstAPI()
-)


Please revert these changes of suit_test.

It seems you don't. Please revert these changes.

worryg0d · 2023-08-30T16:13:30Z

apis/clusterresources/v1beta1/opensearchegressrules_types.go

+
+type OpenSearchEgressRulesSpec struct {
+	ClusterID           string `json:"clusterId"`
+	Name                string `json:"name"`


As Instaclustr API doc says the field name should be returned by the API if you add a new egress rule, but this is not part of a creation request. You should move this field from the resource spec to its status

I've looked at the request sample in the doc, and the name field` is passed there. I think you should test what if you don't pass it. If any error is returned then change nothing

It`s described in spec file, and in request body, in general this field does not affect nothing, but I think we should implement code according to spec

please remove the name field from the spec, it's a read-only property.

worryg0d · 2023-08-30T16:14:44Z

apis/clusterresources/v1beta1/opensearchegressrules_types.go

+
+type OpenSearchEgressRulesStatus struct {
+	ID     string `json:"id,omitempty"`
+	Status string `json:"status,omitempty"`


This field is never used. Please delete it

worryg0d · 2023-08-30T16:21:37Z

controllers/clusterresources/suite_test.go

-var (
-	cfg         *rest.Config
-	k8sClient   client.Client
-	testEnv     *envtest.Environment
-	ctx         context.Context
-	cancel      context.CancelFunc
-	MockInstAPI = mock.NewInstAPI()
-)


It seems you don't. Please revert these changes.

testisnullus · 2023-08-31T15:27:58Z

apis/clusterresources/v1beta1/opensearchegressrules_webhook.go

+	if r.Spec.ClusterID == "" || r.Spec.OpenSearchBindingID == "" || r.Spec.Source == "" {
+		return fmt.Errorf("clusterId, openSearchBindingId, source must be filled")
+	}


Let's remove this code. All these fields are already marked as required in the resource spec

testisnullus · 2023-08-31T15:29:52Z

controllers/clusterresources/opensearchegressrules_controller.go

+	"context"
+	"encoding/json"
+	"errors"
+	"fmt"
+
+	"github.com/go-logr/logr"
+	"github.com/instaclustr/operator/pkg/instaclustr"
+	"github.com/instaclustr/operator/pkg/models"
+	"github.com/instaclustr/operator/pkg/scheduler"
+
+	k8serrors "k8s.io/apimachinery/pkg/api/errors"
+	"k8s.io/apimachinery/pkg/types"
+	"k8s.io/client-go/tools/record"
+	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
+
+	"k8s.io/apimachinery/pkg/runtime"
+	ctrl "sigs.k8s.io/controller-runtime"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/log"
+
+	clusterresourcesv1beta1 "github.com/instaclustr/operator/apis/clusterresources/v1beta1"


Sort imports please

testisnullus · 2023-08-31T15:32:53Z

controllers/clusterresources/opensearchegressrules_controller.go

+	err = json.Unmarshal(b, &rule.Status)
+	if err != nil {
+		l.Error(err, "failed to parse OpenSearch Egress Rule resource response from Instaclustr")
+		r.EventRecorder.Eventf(rule, models.Warning, models.ConvertionFailed,
+			"Failed to parse OpenSearch Egress Rule response from Instaclustr. Reason: %v", err,
+		)
+
+		return err


Let's move unmarshal logic inside the client method

testisnullus · 2023-08-31T15:38:34Z

controllers/clusterresources/opensearchegressrules_controller.go

+	// This hack was added because instaclustr api returns null value egress rule id, but it should be equal to pattern {clusterId}~{source}~{bindingId}
+	// This code could be removed after instaclustr fix bugs, to make code cleaner


// This hack was added because the Instaclustr API returns an egress rule id with a null value, but it should be equal to the pattern {clusterId}~~{source}~~{bindingId}
// This code could be removed after the Instaclustr API team fixes this bug

DoodgeMatvey · 2023-08-31T17:20:23Z

apis/clusterresources/v1beta1/opensearchegressrules_webhook.go

+func (r *OpenSearchEgressRules) ValidateCreate() error {
+	opensearchegressruleslog.Info("validate create", "name", r.Name)
+
+	if r.Spec.ClusterID == "" || r.Spec.OpenSearchBindingID == "" || r.Spec.Source == "" {
+		return fmt.Errorf("clusterId, openSearchBindingId, source must be filled")
+	}


You also need to add enum validation for source: "ALERTING", "NOTIFICATIONS"
And for type: "SLACK", "WEBHOOK", "CUSTOM_WEBHOOK", "CHIME"

I think its unnecessary, all such validations implemented on Instaclustr API, and in case they add new types, logic wouldn't work. About "ALERTING" its already deprecated, I`d rather avoid such validations if it's possible

Yea, but in such way it will be created in k8s but wasn't created in Instaclustr because of error.

DoodgeMatvey · 2023-08-31T17:23:07Z

apis/clusterresources/v1beta1/opensearchegressrules_webhook.go

+// log is for logging in this package.
+var opensearchegressruleslog = logf.Log.WithName("opensearchegressrules-resource")
+
+var openSearchBindingIDPattern, _ = regexp.Compile(`[\w-]+`)


Please add this regular expression to constants

Its only one usage, and its very specific regular expression, I don`t think that it is necessary

Such code should not be hardcoded like this, such constant expressions should be in constants I think

DoodgeMatvey · 2023-08-31T17:24:51Z

apis/clusterresources/v1beta1/opensearchegressrules_webhook.go

+	if r.Spec.ClusterID == "" || r.Spec.OpenSearchBindingID == "" || r.Spec.Source == "" {
+		return fmt.Errorf("clusterId, openSearchBindingId, source must be filled")
+	}


Let's strict to our error naming pattern, so please start it with cannot create OpenSearch Egress Rule

As Danylo sad, this validation is redundant, so I just removed it.
"Let's remove this code. All these fields are already marked as required in the resource spec"

DoodgeMatvey · 2023-08-31T17:30:33Z

apis/clusterresources/v1beta1/opensearchegressrules_webhook.go

+func (r *OpenSearchEgressRules) ValidateUpdate(old runtime.Object) error {
+	opensearchegressruleslog.Info("validate update", "name", r.Name)
+
+	oldRules := old.(*OpenSearchEgressRules)
+
+	if r.Status.ID == "" {
+		return r.ValidateCreate()
+	}
+
+	if r.Spec != oldRules.Spec {
+		return models.ErrImmutableSpec
+	}
+
+	return nil
+}


If I am not wrong there are no API calls to update this entity.

But there are no API calls to update entity, only creation when resource has wrong configuration, to avoid additional operations, like deletion and creation new one, we just can edit resorce and apply it.

How we will update resouce and apply it if there are no such API calls? How we will update it on instaclustr side? In such way you sad k8s will be updated but instaclustr part won't be updated

We need to decline the update of this resource. Without validation, if the customer tries to update this entity inside K8s and this action is passed successfully, it can be really confusing if the entity will not be updated on the Instaclustr API, but updated inside K8s

DoodgeMatvey · 2023-08-31T17:33:28Z

controllers/clusterresources/opensearchegressrules_controller.go

+		return models.ReconcileRequeue, err
+	}
+
+	// Handle resource deletion


Please add TODO for such kind of comments that will be implemented in future

Its not TODO, its just decription to operations, changed it on "It`s handling resource deletion", to add more context

ribaraka · 2023-09-05T08:50:44Z

apis/clusterresources/v1beta1/opensearchegressrules_types.go

+
+type OpenSearchEgressRulesSpec struct {
+	ClusterID           string `json:"clusterId"`
+	Name                string `json:"name"`


please remove the name field from the spec, it's a read-only property.

ribaraka · 2023-09-05T09:18:51Z

controllers/clusterresources/opensearchegressrules_controller.go

+	// This hack was added because the Instaclustr API returns an egress rule id with a null value, but it should be equal to the pattern {clusterId}~{source}~{bindingId}
+	// This code could be removed after the Instaclustr API team fixes this bug
+	if rule.Status.ID == "" {
+		rule.Status.ID = fmt.Sprintf("%s~%s~%s", rule.Spec.ClusterID, rule.Spec.Source, rule.Spec.OpenSearchBindingID)
+	}


ribaraka · 2023-09-05T12:44:16Z

apis/clusterresources/v1beta1/opensearchegressrules_webhook.go

+var theType = []string{"SLACK", "WEBHOOK", "CUSTOM_WEBHOOK", "CHIME"}
+var source = []string{"NOTIFICATIONS"}


theType -> destinationTypes
source -> sourcePlugins
and please add "ALERTING" to the sourcePlugins

ribaraka · 2023-09-05T12:57:10Z

apis/clusterresources/v1beta1/opensearchegressrules_webhook.go

+	equalsToReg, _ := regexp.MatchString(models.OpenSearchBindingIDPattern, r.Spec.OpenSearchBindingID)
+
+	if !slices.Contains(source, r.Spec.Source) || !slices.Contains(theType, r.Spec.Type) {
+		return fmt.Errorf("the source should be equeal to one of options: %q , got: %q. the type should be equeal to one of options: %q , got: %q", source, theType, r.Spec.Source, r.Spec.Type)
+	}
+
+	if !equalsToReg {
+		return fmt.Errorf("mismatching openSearchBindingId to pattern: %s", models.OpenSearchBindingIDPattern)
+	}


please move if !equalsToReg { immediately after the equalsToReg variable is assigned. And add an error

ribaraka · 2023-09-05T13:01:44Z

controllers/clusterresources/opensearchegressrules_controller.go

+	err := r.API.DeleteOpenSearchEgressRule(rule.Status.ID)
+	if err != nil {
+		logger.Error(err, "failed to delete OpenSearch Egress Rule on Instaclustr")


I think it's better to handle the NotFound error

ribaraka · 2023-09-05T13:13:20Z

pkg/instaclustr/client.go

+	err = json.Unmarshal(b, &rule.Status)
+	if err != nil {
+		return err
+	}


It would be clearer if you just returned the bytes and only then converted them to rule.Status. This shows that the status is indeed changing and you are performing the patch operation.

After discussion with the team, we agreed that we should convert data in client

ribaraka · 2023-09-05T13:15:03Z

pkg/instaclustr/interfaces.go

@@ -98,6 +98,8 @@ type API interface {
 	ListAppVersions(app string) ([]*models.AppVersions, error)
 	GetDefaultCredentialsV1(clusterID string) (string, string, error)
 	UpdateClusterSettings(clusterID string, settings *models.ClusterSettings) error
+	CreateOpenSearchEgressRules(rule *clusterresourcesv1beta1.OpenSearchEgressRules) error
+	DeleteOpenSearchEgressRule(egressRuleId string) error


egressRuleId -> ID

ribaraka · 2023-09-05T13:16:00Z

pkg/models/errors.go

@@ -62,4 +62,5 @@ var (
 	ErrPrivateLinkOnlyWithPrivateNetworkCluster = errors.New("private link is available only for private network clusters")
 	ErrPrivateLinkSupportedOnlyForSingleDC      = errors.New("private link is only supported for a single data centre")
 	ErrPrivateLinkSupportedOnlyForAWS           = errors.New("private link is supported only for an AWS cloud provider")
+	ErrImmutableSpec                            = errors.New("spec is immutable")


resource specification is immutable

testisnullus · 2023-10-20T13:45:03Z

controllers/clusterresources/opensearchegressrules_controller.go

+	// This hack was added because the Instaclustr API returns an egress rule id with a null value, but it should be equal to the pattern {clusterId}~{source}~{bindingId}
+	// This code could be removed after the Instaclustr API team fixes this bug


Remove this comment or move it in the right place

testisnullus · 2023-10-20T13:45:39Z

pkg/instaclustr/client.go

+	// https://api.dev.instaclustr.com/cluster-management/v2/resources/applications/opensearch/egress-rules/v2/{egressRuleId}
+	fmt.Println(id)
+	url := c.serverHostname + OpenSearchEgressRulesEndpoint + "/" + id
+	fmt.Println("GOT:", url)
+	fmt.Println("expected: https://api.dev.instaclustr.com/cluster-management/v2/resources/applications/opensearch/egress-rules/v2/{egressRuleId}")


Remove unnecessary code please

worryg0d · 2023-10-24T11:14:31Z

apis/clusterresources/v1beta1/opensearchegressrules_webhook.go

+	if !slices.Contains(sourcePlugins, r.Spec.Source) || !slices.Contains(destinationTypes, r.Spec.Type) {
+		return fmt.Errorf("the source should be equeal to one of options: %q , got: %q. the type should be equeal to one of options: %q , got: %q", sourcePlugins, destinationTypes, r.Spec.Source, r.Spec.Type)
+	}


In my opinion, it will be better if you split it into 2 different conditions because the returning error looks unclear to me.

Suggested change

if !slices.Contains(sourcePlugins, r.Spec.Source) || !slices.Contains(destinationTypes, r.Spec.Type) {

return fmt.Errorf("the source should be equeal to one of options: %q , got: %q. the type should be equeal to one of options: %q , got: %q", sourcePlugins, destinationTypes, r.Spec.Source, r.Spec.Type)

}

if !slices.Contains(sourcePlugins, r.Spec.Source) {

return fmt.Errorf("the source should be equal to one of the options: %q , got: %q", sourcePlugins, r.Spec.Source)

}

if !slices.Contains(destinationTypes, r.Spec.Type) {

return fmt.Errorf("the type should be equal to one of the options: %q , got: %q", destinationTypes, r.Spec.Type

}

worryg0d · 2023-10-24T11:16:47Z

controllers/clusterresources/opensearchegressrules_controller.go

+	"errors"
+	"fmt"
+
+	"github.com/go-logr/logr"


I don't want to say it but anyway someone will say So, please sort imports

worryg0d · 2023-10-24T11:25:25Z

controllers/clusterresources/opensearchegressrules_controller.go

+	patch := rule.NewPatch()
+
+	err = r.Status().Patch(ctx, rule, patch)


It looks a bit weird. As I understand here you patch an ID of the resource. I suggest you create another variable for self-composed ID and provide it anywhere you want. Then patch rule.Status.ID with the value of the created variable.

worryg0d · 2023-10-24T11:27:26Z

pkg/instaclustr/client.go

+	}
+	err = json.Unmarshal(b, &rule.Status)


Suggested change

}

err = json.Unmarshal(b, &rule.Status)

}

err = json.Unmarshal(b, &rule.Status)

worryg0d · 2023-10-24T11:28:13Z

pkg/instaclustr/client.go

+}
+
+func (c *Client) DeleteOpenSearchEgressRule(id string) error {
+	// https://api.dev.instaclustr.com/cluster-management/v2/resources/applications/opensearch/egress-rules/v2/{egressRuleId}


Please delete this

worryg0d · 2023-10-24T11:28:40Z

pkg/instaclustr/client.go

+	}
+	err = json.Unmarshal(b, &rule)


A bit of code cleanup. Also do it in other similar cases if they exists

Suggested change

}

err = json.Unmarshal(b, &rule)

}

err = json.Unmarshal(b, &rule)

ribaraka · 2023-10-25T13:03:28Z

controllers/clusterresources/opensearchegressrules_controller.go

+import (
+	"context"
+	"errors"
+	"fmt"
+
+	clusterresourcesv1beta1 "github.com/instaclustr/operator/apis/clusterresources/v1beta1"
+	"github.com/instaclustr/operator/pkg/instaclustr"
+	"github.com/instaclustr/operator/pkg/models"
+	"github.com/instaclustr/operator/pkg/scheduler"
+
+	"github.com/go-logr/logr"
+	k8serrors "k8s.io/apimachinery/pkg/api/errors"
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/types"
+	"k8s.io/client-go/tools/record"
+	ctrl "sigs.k8s.io/controller-runtime"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
+	"sigs.k8s.io/controller-runtime/pkg/log"
+)


instaclustr/operator imports go third.

ribaraka · 2023-10-25T13:19:53Z

controllers/clusterresources/opensearchegressrules_controller.go

+	if rule.DeletionTimestamp != nil {
+		err = r.handleDelete(ctx, l, rule)
+		if err != nil {
+			return models.ReconcileRequeue, err


please apply the custom rate limiter and remove models.ReconcileRequeue

resolved

OleksiienkoMykyta force-pushed the issue-522-implementation-for-OpenSearch-Egress-Rules-lifecycle-workflow-on-APIv2 branch 4 times, most recently from 93f3c74 to bde9855 Compare August 22, 2023 15:43

OleksiienkoMykyta changed the title ~~issue-522,implementation for OpenSearch Egress Rules lifecycle on APIv2~~ [WIP]issue-522,implementation for OpenSearch Egress Rules lifecycle on APIv2 Aug 23, 2023

OleksiienkoMykyta force-pushed the issue-522-implementation-for-OpenSearch-Egress-Rules-lifecycle-workflow-on-APIv2 branch from bde9855 to 64565f5 Compare August 30, 2023 10:02

OleksiienkoMykyta changed the title ~~[WIP]issue-522,implementation for OpenSearch Egress Rules lifecycle on APIv2~~ issue-522,implementation for OpenSearch Egress Rules lifecycle on APIv2 Aug 30, 2023

OleksiienkoMykyta requested review from testisnullus, ribaraka and worryg0d August 30, 2023 10:02

OleksiienkoMykyta force-pushed the issue-522-implementation-for-OpenSearch-Egress-Rules-lifecycle-workflow-on-APIv2 branch from 64565f5 to 8d82646 Compare August 30, 2023 10:12

OleksiienkoMykyta requested a review from tengu-alt August 30, 2023 10:55

tengu-alt assigned OleksiienkoMykyta Aug 30, 2023

worryg0d requested changes Aug 30, 2023

View reviewed changes

OleksiienkoMykyta force-pushed the issue-522-implementation-for-OpenSearch-Egress-Rules-lifecycle-workflow-on-APIv2 branch from 8d82646 to 6aec1e1 Compare August 30, 2023 12:20

OleksiienkoMykyta requested a review from worryg0d August 30, 2023 12:21

OleksiienkoMykyta force-pushed the issue-522-implementation-for-OpenSearch-Egress-Rules-lifecycle-workflow-on-APIv2 branch from 6aec1e1 to 641c1cb Compare August 30, 2023 12:23

worryg0d requested changes Aug 30, 2023

View reviewed changes

OleksiienkoMykyta force-pushed the issue-522-implementation-for-OpenSearch-Egress-Rules-lifecycle-workflow-on-APIv2 branch from 641c1cb to fadba6d Compare August 31, 2023 11:46

OleksiienkoMykyta requested a review from worryg0d August 31, 2023 11:54

OleksiienkoMykyta force-pushed the issue-522-implementation-for-OpenSearch-Egress-Rules-lifecycle-workflow-on-APIv2 branch from fadba6d to b68c49e Compare August 31, 2023 11:59

worryg0d approved these changes Aug 31, 2023

View reviewed changes

OleksiienkoMykyta force-pushed the issue-522-implementation-for-OpenSearch-Egress-Rules-lifecycle-workflow-on-APIv2 branch from b68c49e to d60d68f Compare August 31, 2023 12:06

tengu-alt approved these changes Aug 31, 2023

View reviewed changes

testisnullus requested changes Aug 31, 2023

View reviewed changes

DoodgeMatvey previously requested changes Aug 31, 2023

View reviewed changes

OleksiienkoMykyta force-pushed the issue-522-implementation-for-OpenSearch-Egress-Rules-lifecycle-workflow-on-APIv2 branch 2 times, most recently from bbbc429 to 6732f3b Compare September 4, 2023 15:46

OleksiienkoMykyta requested review from testisnullus and DoodgeMatvey September 4, 2023 15:47

OleksiienkoMykyta force-pushed the issue-522-implementation-for-OpenSearch-Egress-Rules-lifecycle-workflow-on-APIv2 branch from 6732f3b to 5ed815b Compare September 4, 2023 15:48

ribaraka requested changes Sep 5, 2023

View reviewed changes

testisnullus added the enhancement New feature or request label Sep 13, 2023

OleksiienkoMykyta force-pushed the issue-522-implementation-for-OpenSearch-Egress-Rules-lifecycle-workflow-on-APIv2 branch from 5ed815b to 67d506a Compare October 4, 2023 12:48

OleksiienkoMykyta force-pushed the issue-522-implementation-for-OpenSearch-Egress-Rules-lifecycle-workflow-on-APIv2 branch 2 times, most recently from 00a597d to c49d311 Compare October 20, 2023 13:13

testisnullus approved these changes Oct 20, 2023

View reviewed changes

OleksiienkoMykyta requested a review from ribaraka October 24, 2023 08:16

OleksiienkoMykyta force-pushed the issue-522-implementation-for-OpenSearch-Egress-Rules-lifecycle-workflow-on-APIv2 branch from c49d311 to ef887fb Compare October 24, 2023 08:20

worryg0d reviewed Oct 24, 2023

View reviewed changes

OleksiienkoMykyta force-pushed the issue-522-implementation-for-OpenSearch-Egress-Rules-lifecycle-workflow-on-APIv2 branch from ef887fb to f45c633 Compare October 24, 2023 12:14

ribaraka requested changes Oct 25, 2023

View reviewed changes

ribaraka reviewed Oct 25, 2023

View reviewed changes

OleksiienkoMykyta force-pushed the issue-522-implementation-for-OpenSearch-Egress-Rules-lifecycle-workflow-on-APIv2 branch 2 times, most recently from 37f6b61 to de72397 Compare October 26, 2023 10:11

issue-522,implementation for OpenSearch Egress Rules lifecycle on APIv2

c3559f8

OleksiienkoMykyta force-pushed the issue-522-implementation-for-OpenSearch-Egress-Rules-lifecycle-workflow-on-APIv2 branch from de72397 to c3559f8 Compare October 26, 2023 13:32

ribaraka approved these changes Oct 26, 2023

View reviewed changes

ribaraka merged commit c9cce62 into main Oct 26, 2023
1 check passed

ribaraka mentioned this pull request Oct 31, 2023

Implement an OpenSearch Egress Rules lifecycle workflow on APIv2 #522

Closed

	return fmt.Errorf("spec.ClusterID, spec.OpenSearchBindingId, spec.Source must be filled")
	return fmt.Errorf("spec.ClusterId, spec.ppenSearchBindingId, spec.source must be filled")

		OpenSearchEgressRulesEndpoint = "/cluster-management/v2/resources/applications/opensearch/egress-rules/v2"
		OpenSearchEgressRuleDeleteEndpoint = "/cluster-management/v2/resources/applications/opensearch/egress-rules/v2/"

		// This hack was added because instaclustr api returns null value egress rule id, but it should be equal to pattern {clusterId}~{source}~{bindingId}
		// This code could be removed after instaclustr fix bugs, to make code cleaner

		var theType = []string{"SLACK", "WEBHOOK", "CUSTOM_WEBHOOK", "CHIME"}
		var source = []string{"NOTIFICATIONS"}

		// This hack was added because the Instaclustr API returns an egress rule id with a null value, but it should be equal to the pattern {clusterId}~{source}~{bindingId}
		// This code could be removed after the Instaclustr API team fixes this bug

-	if !slices.Contains(sourcePlugins, r.Spec.Source) || !slices.Contains(destinationTypes, r.Spec.Type) {
-		return fmt.Errorf("the source should be equeal to one of options: %q , got: %q. the type should be equeal to one of options: %q , got: %q", sourcePlugins, destinationTypes, r.Spec.Source, r.Spec.Type)
-	}
+	if !slices.Contains(sourcePlugins, r.Spec.Source) {
+		return fmt.Errorf("the source should be equal to one of the options: %q , got: %q", sourcePlugins, r.Spec.Source)
+	}
+	if  !slices.Contains(destinationTypes, r.Spec.Type) {
+		return fmt.Errorf("the type should be equal to one of the options: %q , got: %q", destinationTypes, r.Spec.Type
+	}

		patch := rule.NewPatch()

		err = r.Status().Patch(ctx, rule, patch)

issue-522,implementation for OpenSearch Egress Rules lifecycle on APIv2 #527

issue-522,implementation for OpenSearch Egress Rules lifecycle on APIv2 #527

Conversation

OleksiienkoMykyta commented Aug 21, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DoodgeMatvey Sep 1, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ribaraka Sep 5, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ribaraka Sep 5, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DoodgeMatvey Sep 1, 2023 •

edited

Loading

ribaraka Sep 5, 2023 •

edited

Loading

ribaraka Sep 5, 2023 •

edited

Loading