Remove duplicate code from enable/disable features & add tests (#361)

nginx · Jul 7, 2023 · 02f86ca · 02f86ca
1 parent a18048a
commit 02f86ca
Show file tree

Hide file tree

Showing 31 changed files with 975 additions and 597 deletions.
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -126,6 +126,13 @@ jobs:
           path: /tmp/nginx-agent-integration-test-api.log
           retention-days: 3
         if: success() || failure()
+      - name: Archive features integration test logs
+        uses: actions/upload-artifact@v3
+        with:
+          name: features-test-log-${{ matrix.container.image }}-${{ matrix.container.version }}
+          path: /tmp/nginx-agent-integration-test-features.log
+          retention-days: 3
+        if: success() || failure()
 
   performance-test:
     name: Performance Tests

diff --git a/Makefile b/Makefile
@@ -207,6 +207,9 @@ integration-test: local-deb-package local-rpm-package local-apk-package
 	PACKAGES_REPO=${OSS_PACKAGES_REPO} INSTALL_FROM_REPO=${INSTALL_FROM_REPO} PACKAGE_NAME=${PACKAGE_NAME} BASE_IMAGE=${BASE_IMAGE} \
 		OS_VERSION=${OS_VERSION} OS_RELEASE=${OS_RELEASE} DOCKER_COMPOSE_FILE="docker-compose-${CONTAINER_OS_TYPE}.yml" \
 		go test -v ./test/integration/api
+	PACKAGES_REPO=${OSS_PACKAGES_REPO} INSTALL_FROM_REPO=${INSTALL_FROM_REPO} PACKAGE_NAME=${PACKAGE_NAME} BASE_IMAGE=${BASE_IMAGE} \
+		OS_VERSION=${OS_VERSION} OS_RELEASE=${OS_RELEASE} DOCKER_COMPOSE_FILE="docker-compose-${CONTAINER_OS_TYPE}.yml" \
+		go test -v ./test/integration/features
 
 test-bench: ## Run benchmark tests
 	cd test/performance && GOWORK=off CGO_ENABLED=0 go test -mod=vendor -count 5 -timeout 2m -bench=. -benchmem metrics_test.go

diff --git a/sdk/client/commander_test.go b/sdk/client/commander_test.go
@@ -45,6 +45,11 @@ var (
 		DirectoryMap: &proto.DirectoryMap{},
 	}
 	grpcServerMutex = &sync.Mutex{}
+	backOffSettings = backoff.BackoffSettings{
+		InitialInterval: 100 * time.Millisecond,
+		MaxInterval:     100 * time.Millisecond,
+		MaxElapsedTime:  30 * time.Second,
+	}
 )
 
 // Positive Test Cases
@@ -222,11 +227,7 @@ func TestCommander_Recv_Reconnect(t *testing.T) {
 	ctx := context.TODO()
 
 	commanderClient := createTestCommanderClient(dialer)
-	commanderClient.WithBackoffSettings(backoff.BackoffSettings{
-		InitialInterval: 100 * time.Millisecond,
-		MaxInterval:     100 * time.Millisecond,
-		MaxElapsedTime:  30 * time.Second,
-	})
+	commanderClient.WithBackoffSettings(backOffSettings)
 	err := commanderClient.Connect(ctx)
 	assert.Nil(t, err)
 
@@ -286,11 +287,7 @@ func TestCommander_Send_Reconnect(t *testing.T) {
 	ctx := context.TODO()
 
 	commanderClient := createTestCommanderClient(dialer)
-	commanderClient.WithBackoffSettings(backoff.BackoffSettings{
-		InitialInterval: 100 * time.Millisecond,
-		MaxInterval:     100 * time.Millisecond,
-		MaxElapsedTime:  30 * time.Second,
-	})
+	commanderClient.WithBackoffSettings(backOffSettings)
 	err := commanderClient.Connect(ctx)
 	assert.Nil(t, err)
 
@@ -339,11 +336,7 @@ func TestCommander_Download_Reconnect(t *testing.T) {
 	ctx := context.TODO()
 
 	commanderClient := createTestCommanderClient(dialer)
-	commanderClient.WithBackoffSettings(backoff.BackoffSettings{
-		InitialInterval: 100 * time.Millisecond,
-		MaxInterval:     100 * time.Millisecond,
-		MaxElapsedTime:  30 * time.Second,
-	})
+	commanderClient.WithBackoffSettings(backOffSettings)
 	err := commanderClient.Connect(ctx)
 	assert.Nil(t, err)
 
@@ -521,11 +514,7 @@ func TestCommander_Upload_Reconnect(t *testing.T) {
 	ctx := context.TODO()
 
 	commanderClient := createTestCommanderClient(dialer)
-	commanderClient.WithBackoffSettings(backoff.BackoffSettings{
-		InitialInterval: 100 * time.Millisecond,
-		MaxInterval:     100 * time.Millisecond,
-		MaxElapsedTime:  30 * time.Second,
-	})
+	commanderClient.WithBackoffSettings(backOffSettings)
 	err := commanderClient.Connect(ctx)
 	assert.Nil(t, err)
 

diff --git a/sdk/config_helpers.go b/sdk/config_helpers.go
@@ -23,6 +23,7 @@ import (
 	"strings"
 	"time"
 
+	"github.com/nginx/agent/sdk/v2/backoff"
 	filesSDK "github.com/nginx/agent/sdk/v2/files"
 	"github.com/nginx/agent/sdk/v2/proto"
 	"github.com/nginx/agent/sdk/v2/zip"
@@ -920,3 +921,24 @@ func GetAppProtectPolicyAndSecurityLogFilesWithIgnoreDirectives(cfg *proto.Nginx
 func GetAppProtectPolicyAndSecurityLogFiles(cfg *proto.NginxConfig) ([]string, []string) {
 	return GetAppProtectPolicyAndSecurityLogFilesWithIgnoreDirectives(cfg, []string{})
 }
+
+func ConvertBackOffSettings(backOffSettings *proto.Backoff) backoff.BackoffSettings {
+	multiplier := backoff.BACKOFF_MULTIPLIER
+	if backOffSettings.GetMultiplier() != 0 {
+		multiplier = backOffSettings.GetMultiplier()
+	}
+
+	jitter := backoff.BACKOFF_JITTER
+	if backOffSettings.GetRandomizationFactor() != 0 {
+		jitter = backOffSettings.GetRandomizationFactor()
+	}
+	cBackoff := backoff.BackoffSettings{
+		InitialInterval: time.Duration(backOffSettings.InitialInterval * int64(time.Second)),
+		MaxInterval:     time.Duration(backOffSettings.MaxInterval * int64(time.Second)),
+		MaxElapsedTime:  time.Duration(backOffSettings.MaxElapsedTime * int64(time.Second)),
+		Multiplier:      multiplier,
+		Jitter:          jitter,
+	}
+
+	return cBackoff
+}
diff --git a/src/core/mock_pipe.go b/src/core/mock_pipe.go
@@ -58,7 +58,27 @@ func (p *MockMessagePipe) Register(size int, plugins []Plugin, extensionPlugins
 	return nil
 }
 
-func (p *MockMessagePipe) DeRegister(plugins []string) error {
+func (p *MockMessagePipe) DeRegister(pluginNames []string) error {
+	var plugins []Plugin
+	for _, name := range pluginNames {
+		for _, plugin := range p.plugins {
+			if plugin.Info().Name() == name {
+				plugins = append(plugins, plugin)
+			}
+		}
+	}
+
+	for _, plugin := range plugins {
+		index := getIndex(plugin.Info().Name(), p.plugins)
+
+		if index != -1 {
+			p.plugins = append(p.plugins[:index], p.plugins[index+1:]...)
+
+			plugin.Close()
+		}
+
+	}
+
 	return nil
 }
 

diff --git a/src/core/pipe.go b/src/core/pipe.go
@@ -200,7 +200,5 @@ func (p *MessagePipe) IsPluginAlreadyRegistered(pluginName string) bool {
 			pluginAlreadyRegistered = true
 		}
 	}
-	log.Infof("pluginName: %v", pluginName)
-	log.Infof("pluginAlreadyRegistred: %v", pluginAlreadyRegistered)
 	return pluginAlreadyRegistered
 }
diff --git a/src/core/pipe_test.go b/src/core/pipe_test.go
@@ -75,3 +75,45 @@ func TestMessagePipe(t *testing.T) {
 
 	plugin.AssertExpectations(t)
 }
+
+func TestPipe_DeRegister(t *testing.T) {
+	plugin := new(testPlugin)
+	plugin.On("Init").Times(1)
+	plugin.On("Close").Times(1)
+
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+
+	messagePipe := SetupMockMessagePipe(t, ctx, []Plugin{plugin}, []ExtensionPlugin{})
+
+	err := messagePipe.DeRegister([]string{*plugin.Info().name})
+
+	assert.NoError(t, err)
+
+	assert.Equal(t, 0, len(messagePipe.GetPlugins()))
+}
+
+func TestPipe_IsPluginAlreadyRegistered(t *testing.T) {
+	plugin := new(testPlugin)
+	plugin.On("Init").Times(1)
+	plugin.On("Close").Times(1)
+
+	ctx, cancel := context.WithCancel(context.Background())
+	pipelineDone := make(chan bool)
+
+	messagePipe := NewMessagePipe(ctx)
+	err := messagePipe.Register(10, []Plugin{plugin}, nil)
+
+	assert.NoError(t, err)
+
+	go func() {
+		messagePipe.Run()
+		pipelineDone <- true
+	}() // for the above call being asynchronous
+
+	cancel()
+	<-pipelineDone
+
+	assert.True(t, messagePipe.IsPluginAlreadyRegistered(*plugin.Info().name))
+	assert.False(t, messagePipe.IsPluginAlreadyRegistered("metrics"))
+}
diff --git a/src/extensions/nginx_app_protect.go b/src/extensions/nginx_app_protect.go
@@ -104,9 +104,7 @@ func (n *NginxAppProtect) Init(pipeline core.MessagePipeInterface) {
 func (n *NginxAppProtect) Process(msg *core.Message) {}
 
 func (n *NginxAppProtect) Subscriptions() []string {
-	return []string{
-		core.AgentConfigChanged,
-	}
+	return []string{}
 }
 
 func (n *NginxAppProtect) Close() {