From 59c0c328171db2db7daea99c3e9277f629d1f25e Mon Sep 17 00:00:00 2001 From: arouco Date: Wed, 4 Dec 2024 17:00:30 -0300 Subject: [PATCH] feat(metricprovider): credentials to download plugin (#3905) * adding logic to defined headers to download plugins from secure sources. Signed-off-by: Ariadna Rouco * adding documentation for plugin download headers Signed-off-by: Ariadna Rouco * removing insignificant line. Signed-off-by: Ariadna Rouco * increasing coverage Signed-off-by: Ariadna Rouco * adding coverage. Signed-off-by: Ariadna Rouco * malformed url case coverage. Signed-off-by: Ariadna Rouco --------- Signed-off-by: Ariadna Rouco --- controller/controller.go | 4 +- controller/controller_test.go | 34 +++++++ docs/analysis/plugins.md | 26 +++++ docs/features/traffic-management/plugins.md | 11 +++ utils/plugin/downloader.go | 45 +++++++-- utils/plugin/downloader_test.go | 101 +++++++++++++++++--- utils/plugin/types/types.go | 10 ++ 7 files changed, 208 insertions(+), 23 deletions(-) diff --git a/controller/controller.go b/controller/controller.go index e9509cd78e..0e7a0a8cae 100644 --- 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 index 0f94131c96..8bd9582fba 100644 --- 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 index 82f63183ad..e1356bf447 100644 --- 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 + 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 index 51d982417a..1d726ff67f 100644 --- 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 + My-Header: value ``` ## List of Available Plugins (alphabetical order) diff --git a/utils/plugin/downloader.go b/utils/plugin/downloader.go index a67dcb38e5..15bf560e1c 100644 --- 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 index f01e1d0e5f..7c54f6b8ef 100644 --- 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) { diff --git a/utils/plugin/types/types.go b/utils/plugin/types/types.go index 11cd8be5e4..e3555a9ae5 100644 --- a/utils/plugin/types/types.go +++ b/utils/plugin/types/types.go @@ -136,4 +136,14 @@ type PluginItem struct { Disabled bool `json:"disabled" yaml:"disabled"` // Args holds command line arguments to initialize the plugin Args []string `json:"args" yaml:"args"` + // HeadersFrom holds the names of secrets where the headers should be pulled from + HeadersFrom []HeadersFrom `json:"headersFrom" yaml:"headersFrom"` +} + +type HeadersFrom struct { + SecretRef SecretRef `json:"secretRef" yaml:"secretRef"` +} + +type SecretRef struct { + Name string `json:"name" yaml:"name"` }