fix(api): deduplicate req from plugin's integration (#4921)

* fix(api): deduplicate req from plugin's integration

Signed-off-by: Yvonnick Esnault <yvonnick.esnault@corp.ovh.com>

contrib/integrations/arsenal/plugin-arsenal/Makefile

@@ -3,13 +3,11 @@
 VERSION := $(if ${CDS_SEMVER},${CDS_SEMVER},snapshot)
 GITHASH := $(if ${GIT_HASH},${GIT_HASH},`git log -1 --format="%H"`)
 BUILDTIME := `date "+%m/%d/%y-%H:%M:%S"`
+CDSCTL := $(if ${CDSCTL},${CDSCTL},cdsctl)
 
 TARGET_DIR = ./dist
 TARGET_NAME = plugin-arsenal
 
-PLUGIN_MANIFEST=`echo plugin.yml`
-export PLUGIN_MANIFEST
-
 define PLUGIN_MANIFEST_BINARY
 os: %os%
 arch: %arch%
@@ -50,12 +48,12 @@ build: $(TARGET_DIR)
 
 publish:
 	@echo "Updating plugin..."
-	cdsctl admin plugins import $(TARGET_DIR)/plugin.yml
+	$(CDSCTL) admin plugins import $(TARGET_DIR)/$(TARGET_NAME).yml
 	@for GOOS in $(TARGET_OS); do \
 		for GOARCH in $(TARGET_ARCH); do \
 			EXTENSION=""; \
 			if test "$$GOOS" = "windows" ; then EXTENSION=".exe"; fi; \
 			echo "Updating plugin binary $(TARGET_NAME)-$$GOOS-$$GOARCH$$EXTENSION"; \
-			cdsctl admin plugins binary-add arsenal-deployment-plugin $(TARGET_DIR)/plugin-$$GOOS-$$GOARCH.yml $(TARGET_DIR)/$(TARGET_NAME)-$$GOOS-$$GOARCH$$EXTENSION; \
+			$(CDSCTL) admin plugins binary-add arsenal-deployment-plugin $(TARGET_DIR)/$(TARGET_NAME)-$$GOOS-$$GOARCH.yml $(TARGET_DIR)/$(TARGET_NAME)-$$GOOS-$$GOARCH$$EXTENSION; \
 		done; \
 	done

contrib/integrations/hello/plugin-hello-deployment/main.go

@@ -32,41 +32,41 @@ Hello deployment plugin must configured as following (content of hello-deploymen
 $ cdsctl admin plugins import hello-deployment-plugin.yml
 
 Build the present binaries and import in CDS (content of hello-deployment-plugin-bin.yml):
-	os: linux
-	arch: amd64
-	cmd: <path-to-binary-file>
+  os: linux
+  arch: amd64
+  cmd: <path-to-binary-file>
 
 $ cdsctl admin plugins binary-add hello-deployment-plugin hello-deployment-plugin-bin.yml <path-to-binary-file>
 
 Hello integration must configured as following (content of hello-integration.yml)
-	name: Hello
-	author: "Username Lastname"
-	default_config: {}
-	deployment_default_config:
-	deployment.token:
-		value: ""
-		type: password
-	retry.delay:
-		value: "10"
-		type: string
-	retry.max:
-		value: "30"
-		type: string
-	version:
-		value: '{{.cds.version}}'
-		type: string
-	plugin: hello-deployment-plugin
-	public_configurations:
-	hello-integration-dev:
-		host:
-		value: http://hello.your-deployment-system.dev.local
-		type: string
-	hello-integration-prod:
-		host:
-		value: http://hello.your-deployment-system.prod.local
-		type: string
-	deployment: true
-	public: true
+  name: Hello
+  author: "Username Lastname"
+  default_config: {}
+  deployment_default_config:
+  deployment.token:
+    value: ""
+    type: password
+  retry.delay:
+    value: "10"
+    type: string
+  retry.max:
+    value: "30"
+    type: string
+  version:
+    value: '{{.cds.version}}'
+    type: string
+  plugin: hello-deployment-plugin
+  public_configurations:
+    hello-integration-dev:
+      host:
+        value: http://hello.your-deployment-system.dev.local
+        type: string
+    hello-integration-prod:
+      host:
+        value: http://hello.your-deployment-system.prod.local
+        type: string
+  deployment: true
+  public: true
 
 $ cdsctl admin integration-model import hello-integration.yml
 

docs/content/docs/integrations/kubernetes/kubernetes_deployment.md

@@ -184,32 +184,30 @@ deployment_default_config:
   helm_values:
     value: ""
     type: string
-    description: specify helm values in a YAML file or a URL to configure/override
-      your helm chart
+    description: specify helm values in a YAML file or a URL to configure/override your helm chart
   namespace:
     value: default
     type: string
-    description: Kubernetes namespace in which you want to deploy your components
-      (OPTIONAL)
+    description: Kubernetes namespace in which you want to deploy your components (OPTIONAL)
   timeout:
     value: "180"
     type: string
     description: timeout in seconds
 public_configurations:
   your-public-myk8s-integration:
     "api_url":
-        value: https://your-k8s.localhost.local
-        type: string
+      value: https://your-k8s.localhost.local
+      type: string
     "ca_certificate":
-        value: |-
-          -----BEGIN CERTIFICATE-----
-          XXX
-          -----END CERTIFICATE-----
-        type: text
-        description: Certificate Authority bundle (PEM format)
+      value: |-
+        -----BEGIN CERTIFICATE-----
+        XXX
+        -----END CERTIFICATE-----
+      type: text
+      description: Certificate Authority bundle (PEM format)
     "token":
-        value: XXX
-        type: string
+      value: XXX
+      type: string
 ```
 
 Import the integration with :

docs/content/docs/integrations/rabbitmq.md

@@ -72,23 +72,23 @@ public_configurations:
       type: password
       value: xxxxxxxx
     "binding_key":
-        type: string
-        value: "test-key"
+      type: string
+      value: "test-key"
     "consumer_tag":
-        type: string
-        value: "simple-consumer"
+      type: string
+      value: "simple-consumer"
     "exchange_name":
-        type: string
-        value: "test-exchange"
+      type: string
+      value: "test-exchange"
     "exchange_type":
-        type: string
-        value: "direct"
+      type: string
+      value: "direct"
     "integration":
-        type: string
-        value: "RabbitMQ"
+      type: string
+      value: "RabbitMQ"
     "queue":
-        type: string
-        value: "test-queue"
+      type: string
+      value: "test-queue"
 ```
 
 Import the integration with :

engine/api/workflow/process_requirements.go

@@ -23,10 +23,17 @@ func processNodeJobRunRequirements(ctx context.Context, db gorp.SqlExecutor, j s
 	var model string
 	var tmp = sdk.ParametersToMap(run.BuildParameters)
 
+	pluginsRequirements := []sdk.Requirement{}
 	for i := range integrationPluginBinaries {
-		j.Action.Requirements = append(j.Action.Requirements, integrationPluginBinaries[i].Requirements...)
+		pluginsRequirements = append(pluginsRequirements, integrationPluginBinaries[i].Requirements...)
 	}
 
+	// as some plugin binaries can have same requirement, we deduplicate them
+	pluginsRequirements = sdk.RequirementListDeduplicate(pluginsRequirements)
+
+	// then add plugins requirement to the action requirement
+	j.Action.Requirements = append(j.Action.Requirements, pluginsRequirements...)
+
 	for _, v := range j.Action.Requirements {
 		name, errName := interpolate.Do(v.Name, tmp)
 		if errName != nil {

engine/worker/internal/requirement.go

@@ -125,13 +125,8 @@ func checkModelRequirement(w *CurrentWorker, r sdk.Requirement) (bool, error) {
 }
 
 func checkNetworkAccessRequirement(w *CurrentWorker, r sdk.Requirement) (bool, error) {
-	conn, err := net.DialTimeout("tcp", r.Value, 10*time.Second)
-	if err != nil {
-		return false, nil
-	}
-	conn.Close()
-
-	return true, nil
+	isValid := sdk.CheckNetworkAccessRequirement(r)
+	return isValid, nil
 }
 
 func checkServiceRequirement(w *CurrentWorker, r sdk.Requirement) (bool, error) {

go.mod

@@ -64,6 +64,7 @@ require (
 	github.com/gocql/gocql v0.0.0-20181018123354-22229812a83e // indirect
 	github.com/golang/mock v1.4.0
 	github.com/golang/protobuf v1.3.2
+	github.com/google/go-cmp v0.4.0
 	github.com/googleapis/gnostic v0.1.0 // indirect
 	github.com/gophercloud/gophercloud v0.0.0-20190504011306-6f9faf57fddc
 	github.com/gopherjs/gopherjs v0.0.0-20181017120253-0766667cb4d1 // indirect

go.sum

@@ -201,6 +201,8 @@ github.com/google/go-cmp v0.2.0/go.mod h1:oXzfMopK8JAjlY9xF4vHSVASa0yLyX7SntLO5a
 github.com/google/go-cmp v0.3.0/go.mod h1:8QqcDgzrUqlUb/G2PQTWiueGozuR1884gddMywk6iLU=
 github.com/google/go-cmp v0.3.1 h1:Xye71clBPdm5HgqGwUkwhbynsUJZhDbS20FvLhQ2izg=
 github.com/google/go-cmp v0.3.1/go.mod h1:8QqcDgzrUqlUb/G2PQTWiueGozuR1884gddMywk6iLU=
+github.com/google/go-cmp v0.4.0 h1:xsAVV57WRhGj6kEIi8ReJzQlHHqcBYCElAvkovg3B/4=
+github.com/google/go-cmp v0.4.0/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
 github.com/google/go-querystring v1.0.0 h1:Xkwi/a1rcvNg1PPYe5vI8GbeBY/jrVuDX5ASuANWTrk=
 github.com/google/go-querystring v1.0.0/go.mod h1:odCYkC5MyYFN7vkCjXpyrEuKhc/BUO6wN/zVPAxq5ck=
 github.com/google/gofuzz v1.0.0 h1:A8PeW59pxE9IoFRqBp37U+mSNaQoZ46F1f0f863XSXw=
@@ -676,6 +678,7 @@ golang.org/x/tools v0.0.0-20190524140312-2c0ae7006135/go.mod h1:RgjU9mgBXZiqYHBn
 golang.org/x/tools v0.0.0-20190606124116-d0a3d012864b/go.mod h1:/rFqwRUd4F7ZHNgwSSTFct+R/Kf4OFW1sUzUTQQTgfc=
 golang.org/x/tools v0.0.0-20190628153133-6cdbf07be9d0 h1:Dh6fw+p6FyRl5x/FvNswO1ji0lIGzm3KP8Y9VkS9PTE=
 golang.org/x/tools v0.0.0-20190628153133-6cdbf07be9d0/go.mod h1:/rFqwRUd4F7ZHNgwSSTFct+R/Kf4OFW1sUzUTQQTgfc=
+golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
 google.golang.org/api v0.3.1/go.mod h1:6wY9I6uQWHQ8EM57III9mq/AjF+i8G65rmVagqKMtkk=
 google.golang.org/api v0.3.2/go.mod h1:6wY9I6uQWHQ8EM57III9mq/AjF+i8G65rmVagqKMtkk=
 google.golang.org/api v0.4.0/go.mod h1:8k5glujaEP+g9n7WNsDg8QP6cUVNI86fCNMcbazEtwE=

sdk/hatchery/hatchery.go

@@ -273,8 +273,13 @@ func canRunJob(ctx context.Context, h Interface, j workerStarterRequest) bool {
 			return false
 		}
 
-		// Skip network access requirement as we can't check it
-		if r.Type == sdk.NetworkAccessRequirement || r.Type == sdk.PluginRequirement || r.Type == sdk.ServiceRequirement || r.Type == sdk.MemoryRequirement {
+		if r.Type == sdk.NetworkAccessRequirement && !sdk.CheckNetworkAccessRequirement(r) {
+			log.Debug("canRunJob> %d - job %d - network requirement failed: %v", j.timestamp, j.id, r.Value)
+			return false
+		}
+
+		// Skip others requirement as we can't check it
+		if r.Type == sdk.PluginRequirement || r.Type == sdk.ServiceRequirement || r.Type == sdk.MemoryRequirement {
 			log.Debug("canRunJob> %d - job %d - job with service, plugin, network or memory requirement. Skip these check as we can't checkt it on hatchery routine", j.timestamp, j.id)
 			continue
 		}
@@ -354,8 +359,13 @@ func canRunJobWithModel(ctx context.Context, h InterfaceWithModels, j workerStar
 			return false
 		}
 
-		// Skip network access requirement as we can't check it
-		if r.Type == sdk.NetworkAccessRequirement || r.Type == sdk.PluginRequirement || r.Type == sdk.ServiceRequirement || r.Type == sdk.MemoryRequirement {
+		if r.Type == sdk.NetworkAccessRequirement && !sdk.CheckNetworkAccessRequirement(r) {
+			log.Debug("canRunJob> %d - job %d - network requirement failed: %v", j.timestamp, j.id, r.Value)
+			return false
+		}
+
+		// Skip other requirement as we can't check it
+		if r.Type == sdk.PluginRequirement || r.Type == sdk.ServiceRequirement || r.Type == sdk.MemoryRequirement {
 			log.Debug("canRunJob> %d - job %d - job with service, plugin, network or memory requirement. Skip these check as we can't check it on hatchery routine", j.timestamp, j.id)
 			continue
 		}

sdk/integration.go

@@ -155,7 +155,7 @@ func DefaultIfEmptyStorage(integrationName string) string {
 	return integrationName
 }
 
-// IntegrationConfig represent the configuration of a plateform
+// IntegrationConfig represent the configuration of an integration
 type IntegrationConfig map[string]IntegrationConfigValue
 
 // Clone return a copy of the config (with a copy of the underlying data structure)

sdk/requirement.go

@@ -1,5 +1,10 @@
 package sdk
 
+import (
+	"net"
+	"time"
+)
+
 const (
 	//BinaryRequirement refers to the need to a specific binary on host running the action
 	BinaryRequirement = "binary"
@@ -33,6 +38,21 @@ func (l RequirementList) Values() []string {
 	return values
 }
 
+// RequirementListDeduplicate returns requirements list without duplicate values.
+func RequirementListDeduplicate(l RequirementList) RequirementList {
+	m := map[string]Requirement{}
+
+	for i := range l {
+		m[l[i].Name+l[i].Type+l[i].Value] = l[i]
+	}
+
+	newList := make([]Requirement, 0, len(m))
+	for i := range m {
+		newList = append(newList, m[i])
+	}
+	return newList
+}
+
 // IsValid returns requirement list validity.
 func (l RequirementList) IsValid() error {
 	// check requirement unicity
@@ -155,3 +175,14 @@ func (a *Action) Requirement(name string, t string, value string) *Action {
 	a.Requirements = append(a.Requirements, r)
 	return a
 }
+
+// CheckNetworkAccessRequirement returns true if req.Value can Dial
+func CheckNetworkAccessRequirement(req Requirement) bool {
+	conn, err := net.DialTimeout("tcp", req.Value, 10*time.Second)
+	if err != nil {
+		return false
+	}
+	conn.Close()
+
+	return true
+}