feat(metricprovider): credentials to download plugin (argoproj#3905)

* adding logic to defined headers to download plugins from secure sources. Signed-off-by: Ariadna Rouco <[email protected]> * adding documentation for plugin download headers Signed-off-by: Ariadna Rouco <[email protected]> * removing insignificant line. Signed-off-by: Ariadna Rouco <[email protected]> * increasing coverage Signed-off-by: Ariadna Rouco <[email protected]> * adding coverage. Signed-off-by: Ariadna Rouco <[email protected]> * malformed url case coverage. Signed-off-by: Ariadna Rouco <[email protected]> --------- Signed-off-by: Ariadna Rouco <[email protected]>
Rizwana777 · Dec 12, 2024 · 59c0c32 · 59c0c32
1 parent d9756c6
commit 59c0c32
Show file tree

Hide file tree

Showing 7 changed files with 208 additions and 23 deletions.
diff --git a/controller/controller.go b/controller/controller.go
@@ -238,7 +238,7 @@ func NewAnalysisManager(
 		log.Fatalf("Failed to init config: %v", err)
 	}
 
-	err = plugin.DownloadPlugins(plugin.FileDownloaderImpl{})
+	err = plugin.DownloadPlugins(plugin.FileDownloaderImpl{}, kubeclientset)
 	if err != nil {
 		log.Fatalf("Failed to download plugins: %v", err)
 	}
@@ -444,7 +444,7 @@ func NewManager(
 		log.Fatalf("Failed to init config: %v", err)
 	}
 
-	err = plugin.DownloadPlugins(plugin.FileDownloaderImpl{})
+	err = plugin.DownloadPlugins(plugin.FileDownloaderImpl{}, kubeclientset)
 	if err != nil {
 		log.Fatalf("Failed to download plugins: %v", err)
 	}

diff --git a/controller/controller_test.go b/controller/controller_test.go
@@ -274,6 +274,40 @@ func TestNewManager(t *testing.T) {
 	assert.NotNil(t, cm)
 }
 
+func TestNewAnalysisManager(t *testing.T) {
+	f := newFixture(t)
+
+	i := informers.NewSharedInformerFactory(f.client, noResyncPeriodFunc())
+	k8sI := kubeinformers.NewSharedInformerFactory(f.kubeclient, noResyncPeriodFunc())
+
+	scheme := runtime.NewScheme()
+	listMapping := map[schema.GroupVersionResource]string{}
+	dynamicClient := dynamicfake.NewSimpleDynamicClientWithCustomListKinds(scheme, listMapping)
+	dynamicInformerFactory := dynamicinformer.NewDynamicSharedInformerFactory(dynamicClient, 0)
+
+	k8sRequestProvider := &metrics.K8sRequestsCountProvider{}
+	cm := NewAnalysisManager(
+		"default",
+		f.kubeclient,
+		f.client,
+		k8sI.Batch().V1().Jobs(),
+		i.Argoproj().V1alpha1().AnalysisRuns(),
+		i.Argoproj().V1alpha1().AnalysisTemplates(),
+		i.Argoproj().V1alpha1().ClusterAnalysisTemplates(),
+		noResyncPeriodFunc(),
+		8090,
+		8080,
+		k8sRequestProvider,
+		nil,
+		dynamicInformerFactory,
+		false,
+		nil,
+		nil,
+	)
+
+	assert.NotNil(t, cm)
+}
+
 func TestPrimaryController(t *testing.T) {
 	f := newFixture(t)
 

diff --git a/docs/analysis/plugins.md b/docs/analysis/plugins.md
@@ -65,8 +65,34 @@ data:
     - name: "argoproj-labs/sample-prometheus" # name of the plugin, it must match the name required by the plugin so it can find it's configuration
       location: "https://github.com/argoproj-labs/rollouts-plugin-metric-sample-prometheus/releases/download/v0.0.4/metric-plugin-linux-amd64" # supports http(s):// urls and file://
       sha256: "dac10cbf57633c9832a17f8c27d2ca34aa97dd3d" #optional sha256 checksum of the plugin executable
+      headersFrom: #optional headers for the download via http request 
+        - secretRef:
+            name: secret-name
+---
+apiVersion: v1
+kind: Secret
+metadata:
+  name: secret-name
+stringData:
+  Authorization: Basic <Base 64 TOKEN>
+  My-Header: value
 ```
 
+
+
+## Some words of caution
+
+Depending on which method you use to install and the plugin, there are some things to be aware of.
+The rollouts controller will not start if it can not download or find the plugin executable. This means that if you are using
+a method of installation that requires a download of the plugin and the server hosting the plugin for some reason is not available and the rollouts
+controllers pod got deleted while the server was down or is coming up for the first time, it will not be able to start until
+the server hosting the plugin is available again.
+
+Argo Rollouts will download the plugin at startup only once but if the pod is deleted it will need to download the plugin again on next startup. Running
+Argo Rollouts in HA mode can help a little with this situation because each pod will download the plugin at startup. So if a single pod gets
+deleted during a server outage, the other pods will still be able to take over because there will already be a plugin executable available to it. It is the
+responsibility of the Argo Rollouts administrator to define the plugin installation method considering the risks of each approach.
+
 ## List of Available Plugins (alphabetical order)
 
 If you have created a plugin, please submit a PR to add it to this list.

diff --git a/docs/features/traffic-management/plugins.md b/docs/features/traffic-management/plugins.md
@@ -65,6 +65,17 @@ data:
     - name: "argoproj-labs/sample-nginx" # name of the plugin, it must match the name required by the plugin so it can find it's configuration
       location: "https://github.com/argoproj-labs/rollouts-plugin-trafficrouter-sample-nginx/releases/download/v0.0.1/metric-plugin-linux-amd64" # supports http(s):// urls and file://
       sha256: "08f588b1c799a37bbe8d0fc74cc1b1492dd70b2c" #optional sha256 checksum of the plugin executable
+      headersFrom: #optional headers for the download via http request 
+        - secretRef:
+            name: secret-name
+---
+apiVersion: v1
+kind: Secret
+metadata:
+  name: secret-name
+stringData:
+  Authorization: Basic <Base 64 TOKEN>
+  My-Header: value
 ```
 
 ## List of Available Plugins (alphabetical order)

diff --git a/utils/plugin/downloader.go b/utils/plugin/downloader.go
@@ -1,6 +1,7 @@
 package plugin
 
 import (
+	"context"
 	"crypto/sha256"
 	"fmt"
 	"io"
@@ -11,6 +12,8 @@ import (
 	"time"
 
 	argoConfig "github.com/argoproj/argo-rollouts/utils/config"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/client-go/kubernetes"
 
 	"github.com/argoproj/argo-rollouts/utils/defaults"
 
@@ -19,15 +22,20 @@ import (
 
 // FileDownloader is an interface that allows us to mock the http.Get function
 type FileDownloader interface {
-	Get(url string) (resp *http.Response, err error)
+	Get(url string, header http.Header) (resp *http.Response, err error)
 }
 
 // FileDownloaderImpl is the default/real implementation of the FileDownloader interface
 type FileDownloaderImpl struct {
 }
 
-func (fd FileDownloaderImpl) Get(url string) (resp *http.Response, err error) {
-	return http.Get(url)
+func (fd FileDownloaderImpl) Get(url string, header http.Header) (resp *http.Response, err error) {
+	request, err := http.NewRequest(http.MethodGet, url, nil)
+	if err != nil {
+		return nil, err
+	}
+	request.Header = header
+	return http.DefaultClient.Do(request)
 }
 
 // checkPluginExists this function checks if the plugin exists in the configured path on the filesystem
@@ -61,12 +69,17 @@ func checkShaOfPlugin(pluginLocation string, expectedSha256 string) (bool, error
 	return match, nil
 }
 
-func downloadFile(filepath string, url string, downloader FileDownloader) error {
-	// Get the data
-	resp, err := downloader.Get(url)
+func downloadFile(filepath string, url string, downloader FileDownloader, header http.Header) error {
+	// Get the data with credentials
+	resp, err := downloader.Get(url, header)
 	if err != nil {
 		return fmt.Errorf("failed to download file from %s: %w", url, err)
 	}
+
+	if isFailure(resp.StatusCode) {
+		return fmt.Errorf("failed to download file from %s: response code %s", url, http.StatusText(resp.StatusCode))
+	}
+
 	defer resp.Body.Close()
 
 	// Create the file
@@ -92,7 +105,7 @@ func downloadFile(filepath string, url string, downloader FileDownloader) error
 }
 
 // DownloadPlugins this function downloads and/or checks that a plugin executable exits on the filesystem
-func DownloadPlugins(fd FileDownloader) error {
+func DownloadPlugins(fd FileDownloader, kubeClient kubernetes.Interface) error {
 	config, err := argoConfig.GetConfig()
 	if err != nil {
 		return fmt.Errorf("failed to get config: %w", err)
@@ -126,7 +139,18 @@ func DownloadPlugins(fd FileDownloader) error {
 		case "http", "https":
 			log.Infof("Downloading plugin %s from: %s", plugin.Name, plugin.Location)
 			startTime := time.Now()
-			err = downloadFile(finalFileLocation, urlObj.String(), fd)
+			requestHeader := http.Header{}
+			for _, header := range plugin.HeadersFrom {
+				secret, err := kubeClient.CoreV1().Secrets(defaults.Namespace()).Get(context.Background(), header.SecretRef.Name, metav1.GetOptions{})
+				if err != nil {
+					return fmt.Errorf("failed to get secret in secretRef: %w", err)
+				}
+				for k, v := range secret.Data {
+					requestHeader.Add(k, string(v))
+				}
+			}
+
+			err = downloadFile(finalFileLocation, urlObj.String(), fd, requestHeader)
 			if err != nil {
 				return fmt.Errorf("failed to download plugin from %s: %w", plugin.Location, err)
 			}
@@ -201,3 +225,8 @@ func copyFile(src, dst string) error {
 	}
 	return nil
 }
+
+// isFailure determines if the response has a 2xx response
+func isFailure(statusCode int) bool {
+	return statusCode < http.StatusOK || statusCode >= http.StatusBadRequest
+}
diff --git a/utils/plugin/downloader_test.go b/utils/plugin/downloader_test.go
@@ -21,7 +21,18 @@ type MockFileDownloader struct {
 	FileDownloader
 }
 
-func (m MockFileDownloader) Get(url string) (*http.Response, error) {
+func (m MockFileDownloader) Get(url string, header http.Header) (*http.Response, error) {
+	if url == "https://test/plugin/fail" {
+		return &http.Response{
+			Status:        "404",
+			StatusCode:    404,
+			Proto:         "HTTP/1.1",
+			ProtoMajor:    1,
+			ProtoMinor:    1,
+			Header:        nil,
+			ContentLength: 4,
+		}, nil
+	}
 	responseBody := io.NopCloser(bytes.NewReader([]byte(`test`)))
 	return &http.Response{
 		Status:        "200",
@@ -47,16 +58,23 @@ func TestPlugin(t *testing.T) {
 				Name:      "argo-rollouts-config",
 				Namespace: "argo-rollouts",
 			},
-			Data: map[string]string{"metricProviderPlugins": "\n  - name: argoproj-labs/http\n    location: https://test/plugin\n  - name: argoproj-labs/http-sha\n    location: https://test/plugin\n    sha256: 74657374e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855\n  - name: argoproj-labs/http-sha-correct\n    location: https://test/plugin\n    sha256: 9f86d081884c7d659a2feaa0c55ad015a3bf4f1b2b0b822cd15d6c15b0f00a08"},
+			Data: map[string]string{"metricProviderPlugins": "\n  - name: argoproj-labs/http\n    location: https://test/plugin\n  - name: argoproj-labs/http-sha\n    location: https://test/plugin\n    sha256: 74657374e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855\n  - name: argoproj-labs/http-sha-correct\n    location: https://test/plugin\n    sha256: 9f86d081884c7d659a2feaa0c55ad015a3bf4f1b2b0b822cd15d6c15b0f00a08\n  - name: argoproj-labs/http-headers-correct\n    location: https://test/plugin\n    headersFrom:\n      - secretRef:\n          name: secret-name"},
 		}
-		client := fake.NewSimpleClientset(cm)
+		secret := &v1.Secret{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "secret-name",
+				Namespace: "argo-rollouts",
+			},
+			Data: map[string][]byte{"Authorization": []byte("Basic VE9LRU4=")},
+		}
+		client := fake.NewSimpleClientset(cm, secret)
 
 		config.UnInitializeConfig()
 
 		_, err := config.InitializeConfig(client, "argo-rollouts-config")
 		assert.NoError(t, err)
 
-		err = DownloadPlugins(MockFileDownloader{})
+		err = DownloadPlugins(MockFileDownloader{}, client)
 		assert.NoError(t, err)
 
 		dir, filename, err := config.GetPluginDirectoryAndFilename("argoproj-labs/http")
@@ -70,6 +88,54 @@ func TestPlugin(t *testing.T) {
 
 		err = os.Remove(filepath.Join(defaults.DefaultRolloutPluginFolder, dir, filename))
 		assert.NoError(t, err)
+
+		dir, filename, err = config.GetPluginDirectoryAndFilename("argoproj-labs/http-headers-correct")
+		assert.NoError(t, err)
+
+		err = os.Remove(filepath.Join(defaults.DefaultRolloutPluginFolder, dir, filename))
+		assert.NoError(t, err)
+		err = os.RemoveAll(defaults.DefaultRolloutPluginFolder)
+		assert.NoError(t, err)
+	})
+
+	t.Run("test failed download", func(t *testing.T) {
+		cm := &v1.ConfigMap{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "argo-rollouts-config",
+				Namespace: "argo-rollouts",
+			},
+			Data: map[string]string{"metricProviderPlugins": "\n  - name: argoproj-labs/http-fail\n    location: https://test/plugin/fail\n"},
+		}
+		client := fake.NewSimpleClientset(cm)
+
+		config.UnInitializeConfig()
+
+		_, err := config.InitializeConfig(client, "argo-rollouts-config")
+		assert.NoError(t, err)
+
+		err = DownloadPlugins(MockFileDownloader{}, client)
+		assert.Error(t, err)
+		err = os.RemoveAll(defaults.DefaultRolloutPluginFolder)
+		assert.NoError(t, err)
+	})
+
+	t.Run("test failed finding secret for header", func(t *testing.T) {
+		cm := &v1.ConfigMap{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "argo-rollouts-config",
+				Namespace: "argo-rollouts",
+			},
+			Data: map[string]string{"metricProviderPlugins": "  - name: argoproj-labs/http-headers-correct\n    location: https://test/plugin\n    headersFrom:\n      - secretRef:\n          name: secret-name"},
+		}
+		client := fake.NewSimpleClientset(cm)
+
+		config.UnInitializeConfig()
+
+		_, err := config.InitializeConfig(client, "argo-rollouts-config")
+		assert.NoError(t, err)
+
+		err = DownloadPlugins(MockFileDownloader{}, client)
+		assert.Error(t, err)
 		err = os.RemoveAll(defaults.DefaultRolloutPluginFolder)
 		assert.NoError(t, err)
 	})
@@ -89,7 +155,7 @@ func TestPlugin(t *testing.T) {
 		_, err := config.InitializeConfig(client, defaults.DefaultRolloutsConfigMapName)
 		assert.NoError(t, err)
 
-		err = DownloadPlugins(MockFileDownloader{})
+		err = DownloadPlugins(MockFileDownloader{}, client)
 		assert.Error(t, err)
 
 		dir, filename, err := config.GetPluginDirectoryAndFilename("argoproj-labs/http-badsha")
@@ -109,7 +175,7 @@ func TestPlugin(t *testing.T) {
 		assert.NoError(t, err)
 		assert.Equal(t, 0, len(cm.GetAllPlugins()))
 
-		err = DownloadPlugins(MockFileDownloader{})
+		err = DownloadPlugins(MockFileDownloader{}, client)
 		assert.NoError(t, err)
 		err = os.RemoveAll(defaults.DefaultRolloutPluginFolder)
 		assert.NoError(t, err)
@@ -130,7 +196,7 @@ func TestPlugin(t *testing.T) {
 		_, err := config.InitializeConfig(client, defaults.DefaultRolloutsConfigMapName)
 		assert.NoError(t, err)
 
-		err = DownloadPlugins(MockFileDownloader{})
+		err = DownloadPlugins(MockFileDownloader{}, client)
 		assert.NoError(t, err)
 
 		dir, filename, err := config.GetPluginDirectoryAndFilename("argoproj-labs/file-plugin")
@@ -159,7 +225,7 @@ func TestPlugin(t *testing.T) {
 		_, err = config.InitializeConfig(client, defaults.DefaultRolloutsConfigMapName)
 		assert.NoError(t, err)
 
-		err = DownloadPlugins(MockFileDownloader{})
+		err = DownloadPlugins(MockFileDownloader{}, client)
 		assert.NoError(t, err)
 
 		dir, filename, err := config.GetPluginDirectoryAndFilename("namespace/file-plugin")
@@ -186,7 +252,7 @@ func TestPlugin(t *testing.T) {
 		_, err := config.InitializeConfig(client, "argo-rollouts-config")
 		assert.Error(t, err)
 
-		err = DownloadPlugins(MockFileDownloader{})
+		err = DownloadPlugins(MockFileDownloader{}, client)
 		assert.Error(t, err)
 	})
 
@@ -205,7 +271,7 @@ func TestPlugin(t *testing.T) {
 		_, err := config.InitializeConfig(client, "argo-rollouts-config")
 		assert.NoError(t, err)
 
-		err = DownloadPlugins(MockFileDownloader{})
+		err = DownloadPlugins(MockFileDownloader{}, client)
 		assert.Error(t, err)
 
 		err = os.RemoveAll(defaults.DefaultRolloutPluginFolder)
@@ -251,9 +317,18 @@ func TestCheckShaOfPlugin(t *testing.T) {
 }
 
 func TestDownloadFile(t *testing.T) {
-	err := downloadFile("error", "", FileDownloaderImpl{})
-	assert.Error(t, err)
-	assert.Contains(t, err.Error(), "failed to download file from")
+	t.Run("test sha of real file", func(t *testing.T) {
+		err := downloadFile("error", " ", FileDownloaderImpl{}, nil)
+		assert.Error(t, err)
+		assert.Contains(t, err.Error(), "failed to download file from")
+	})
+
+	t.Run("test download fail with invalid url", func(t *testing.T) {
+		url := "://example.com"
+		err := downloadFile("error", url, FileDownloaderImpl{}, nil)
+		assert.Error(t, err)
+		assert.Contains(t, err.Error(), "failed to download file from")
+	})
 }
 
 func Test_copyFile(t *testing.T) {