From b73cad735c4f66b812c30b108fa0910d5bb154cb Mon Sep 17 00:00:00 2001 From: Alex Collins Date: Fri, 7 Jun 2019 16:18:16 -0700 Subject: [PATCH] Forward git credentials to config management plugins. Closes #1628 --- controller/appcontroller.go | 6 +++ reposerver/repository/repository.go | 21 ++++++---- reposerver/repository/repository_test.go | 8 +++- test/e2e/app_management_test.go | 20 ++------- test/e2e/custom_tool_test.go | 53 ++++++++++++++++++++++++ test/e2e/fixture/app/actions.go | 4 ++ test/e2e/fixture/app/consequences.go | 4 ++ test/e2e/fixture/app/context.go | 32 ++++++++++---- test/e2e/fixture/fixture.go | 46 +++++++++++++++----- test/e2e/fixture/util.go | 11 +++-- util/git/creds.go | 6 +++ util/kustomize/kustomize.go | 10 ++--- util/kustomize/kustomize_test.go | 3 +- util/settings/settings.go | 10 +++++ 14 files changed, 178 insertions(+), 56 deletions(-) create mode 100644 test/e2e/custom_tool_test.go create mode 100644 util/git/creds.go diff --git a/controller/appcontroller.go b/controller/appcontroller.go index 3088f9ec8e3ad..f4dcb5fd81d19 100644 --- a/controller/appcontroller.go +++ b/controller/appcontroller.go @@ -972,6 +972,7 @@ func (ctrl *ApplicationController) watchSettings(ctx context.Context) { prevAppLabelKey := ctrl.settings.GetAppInstanceLabelKey() prevResourceExclusions := ctrl.settings.ResourceExclusions prevResourceInclusions := ctrl.settings.ResourceInclusions + prevConfigManagementPlugins := ctrl.settings.ConfigManagementPlugins done := false for !done { select { @@ -993,6 +994,11 @@ func (ctrl *ApplicationController) watchSettings(ctx context.Context) { ctrl.stateCache.Invalidate() prevResourceInclusions = newSettings.ResourceInclusions } + if !reflect.DeepEqual(prevConfigManagementPlugins, newSettings.ConfigManagementPlugins) { + log.WithFields(log.Fields{"prevConfigManagementPlugins": prevConfigManagementPlugins, "newConfigManagementPlugins": newSettings.ConfigManagementPlugins}).Info("config management plugins modified") + ctrl.stateCache.Invalidate() + prevConfigManagementPlugins = newSettings.ConfigManagementPlugins + } case <-ctx.Done(): done = true } diff --git a/reposerver/repository/repository.go b/reposerver/repository/repository.go index 60c4c20e2ded8..dde701883fa2a 100644 --- a/reposerver/repository/repository.go +++ b/reposerver/repository/repository.go @@ -14,7 +14,7 @@ import ( "github.com/TomOnTime/utfutil" argoexec "github.com/argoproj/pkg/exec" "github.com/ghodss/yaml" - jsonnet "github.com/google/go-jsonnet" + "github.com/google/go-jsonnet" log "github.com/sirupsen/logrus" "golang.org/x/sync/semaphore" "google.golang.org/grpc/codes" @@ -191,6 +191,7 @@ func GenerateManifests(appPath string, q *ManifestRequest) (*ManifestResponse, e var dest *v1alpha1.ApplicationDestination appSourceType, err := GetAppSourceType(q.ApplicationSource, appPath) + creds := newCreds(q.Repo) switch appSourceType { case v1alpha1.ApplicationSourceTypeKsonnet: targetObjs, dest, err = ksShow(q.AppLabelKey, appPath, q.ApplicationSource.Ksonnet) @@ -216,10 +217,10 @@ func GenerateManifests(appPath string, q *ManifestRequest) (*ManifestResponse, e } } case v1alpha1.ApplicationSourceTypeKustomize: - k := kustomize.NewKustomizeApp(appPath, kustomizeCredentials(q.Repo)) + k := kustomize.NewKustomizeApp(appPath, creds) targetObjs, _, _, err = k.Build(q.ApplicationSource.Kustomize) case v1alpha1.ApplicationSourceTypePlugin: - targetObjs, err = runConfigManagementPlugin(appPath, q, q.Plugins) + targetObjs, err = runConfigManagementPlugin(appPath, q, creds, q.Plugins) case v1alpha1.ApplicationSourceTypeDirectory: var directory *v1alpha1.ApplicationSourceDirectory if directory = q.ApplicationSource.Directory; directory == nil { @@ -511,7 +512,7 @@ func runCommand(command v1alpha1.Command, path string, env []string) (string, er return argoexec.RunCommandExt(cmd) } -func runConfigManagementPlugin(appPath string, q *ManifestRequest, plugins []*v1alpha1.ConfigManagementPlugin) ([]*unstructured.Unstructured, error) { +func runConfigManagementPlugin(appPath string, q *ManifestRequest, creds *git.Creds, plugins []*v1alpha1.ConfigManagementPlugin) ([]*unstructured.Unstructured, error) { var plugin *v1alpha1.ConfigManagementPlugin for i := range plugins { if plugins[i].Name == q.ApplicationSource.Plugin.Name { @@ -523,6 +524,12 @@ func runConfigManagementPlugin(appPath string, q *ManifestRequest, plugins []*v1 return nil, fmt.Errorf("Config management plugin with name '%s' is not supported.", q.ApplicationSource.Plugin.Name) } env := append(os.Environ(), fmt.Sprintf("%s=%s", PluginEnvAppName, q.AppLabelValue), fmt.Sprintf("%s=%s", PluginEnvAppNamespace, q.Namespace)) + if creds != nil { + env = append(env, + "GIT_ASKPASS=git-ask-pass.sh", + fmt.Sprintf("GIT_USERNAME=%s", creds.Username), + fmt.Sprintf("GIT_PASSWORD=%s", creds.Password)) + } if plugin.Init != nil { _, err := runCommand(*plugin.Init, appPath, env) if err != nil { @@ -641,7 +648,7 @@ func (s *Service) GetAppDetails(ctx context.Context, q *RepoServerAppDetailsQuer case v1alpha1.ApplicationSourceTypeKustomize: res.Kustomize = &KustomizeAppSpec{} res.Kustomize.Path = q.Path - k := kustomize.NewKustomizeApp(appPath, kustomizeCredentials(q.Repo)) + k := kustomize.NewKustomizeApp(appPath, newCreds(q.Repo)) _, imageTags, images, err := k.Build(nil) if err != nil { return nil, err @@ -667,11 +674,11 @@ func (q *RepoServerAppDetailsQuery) valueFiles() []string { return q.Helm.ValueFiles } -func kustomizeCredentials(repo *v1alpha1.Repository) *kustomize.GitCredentials { +func newCreds(repo *v1alpha1.Repository) *git.Creds { if repo == nil || repo.Password == "" { return nil } - return &kustomize.GitCredentials{ + return &git.Creds{ Username: repo.Username, Password: repo.Password, } diff --git a/reposerver/repository/repository_test.go b/reposerver/repository/repository_test.go index 73428944854a7..80d09bf5e4b50 100644 --- a/reposerver/repository/repository_test.go +++ b/reposerver/repository/repository_test.go @@ -160,9 +160,12 @@ func TestRunCustomTool(t *testing.T) { Name: "test", Generate: argoappv1.Command{ Command: []string{"sh", "-c"}, - Args: []string{`echo "{\"kind\": \"FakeObject\", \"metadata\": { \"name\": \"$ARGOCD_APP_NAME\", \"namespace\": \"$ARGOCD_APP_NAMESPACE\"}}"`}, + Args: []string{`echo "{\"kind\": \"FakeObject\", \"metadata\": { \"name\": \"$ARGOCD_APP_NAME\", \"namespace\": \"$ARGOCD_APP_NAMESPACE\", \"annotations\": {\"GIT_ASKPASS\": \"$GIT_ASKPASS\", \"GIT_USERNAME\": \"$GIT_USERNAME\", \"GIT_PASSWORD\": \"$GIT_PASSWORD\"}}}"`}, }, }}, + Repo: &argoappv1.Repository{ + Username: "foo", Password: "bar", + }, }) assert.Nil(t, err) @@ -173,6 +176,9 @@ func TestRunCustomTool(t *testing.T) { assert.Equal(t, obj.GetName(), "test-app") assert.Equal(t, obj.GetNamespace(), "test-namespace") + assert.Equal(t, "git-ask-pass.sh", obj.GetAnnotations()["GIT_ASKPASS"]) + assert.Equal(t, "foo", obj.GetAnnotations()["GIT_USERNAME"]) + assert.Equal(t, "bar", obj.GetAnnotations()["GIT_PASSWORD"]) } func TestGenerateFromUTF16(t *testing.T) { diff --git a/test/e2e/app_management_test.go b/test/e2e/app_management_test.go index c5a7c57b4a9dd..3aec0d30a2c6a 100644 --- a/test/e2e/app_management_test.go +++ b/test/e2e/app_management_test.go @@ -284,16 +284,9 @@ func TestResourceDiffing(t *testing.T) { And(func(app *Application) { diffOutput, _ := fixture.RunCli("app", "diff", app.Name, "--local", "testdata/guestbook") assert.Contains(t, diffOutput, fmt.Sprintf("===== apps/Deployment %s/guestbook-ui ======", fixture.DeploymentNamespace())) - - // Update settings to ignore image difference - settings, err := fixture.SettingsManager.GetSettings() - assert.NoError(t, err) - settings.ResourceOverrides = map[string]ResourceOverride{ - "apps/Deployment": {IgnoreDifferences: ` jsonPointers: ["/spec/template/spec/containers/0/image"]`}, - } - err = fixture.SettingsManager.SaveSettings(settings) - assert.NoError(t, err) }). + Given(). + ResourceOverrides(map[string]ResourceOverride{"apps/Deployment": {IgnoreDifferences: ` jsonPointers: ["/spec/template/spec/containers/0/image"]`}}). When(). Refresh(RefreshTypeNormal). Then(). @@ -389,14 +382,7 @@ definitions: func TestResourceAction(t *testing.T) { Given(t). Path(guestbookPath). - And(func() { - settings, err := fixture.SettingsManager.GetSettings() - assert.NoError(t, err) - - settings.ResourceOverrides = map[string]ResourceOverride{"apps/Deployment": {Actions: actionsConfig}} - err = fixture.SettingsManager.SaveSettings(settings) - assert.NoError(t, err) - }). + ResourceOverrides(map[string]ResourceOverride{"apps/Deployment": {Actions: actionsConfig}}). When(). Create(). Sync(). diff --git a/test/e2e/custom_tool_test.go b/test/e2e/custom_tool_test.go new file mode 100644 index 0000000000000..ea0b6852420f0 --- /dev/null +++ b/test/e2e/custom_tool_test.go @@ -0,0 +1,53 @@ +package e2e + +import ( + "testing" + + "github.com/stretchr/testify/assert" + + . "github.com/argoproj/argo-cd/errors" + . "github.com/argoproj/argo-cd/pkg/apis/application/v1alpha1" + . "github.com/argoproj/argo-cd/test/e2e/fixture" + . "github.com/argoproj/argo-cd/test/e2e/fixture/app" +) + +// make sure we can echo back the Git creds +func TestCustomToolWithGitCreds(t *testing.T) { + Given(t). + // path does not matter, we ignore it + ConfigManagementPlugin( + ConfigManagementPlugin{ + Name: Name(), + Generate: Command{ + Command: []string{"sh", "-c"}, + Args: []string{`echo "{\"kind\": \"ConfigMap\", \"apiVersion\": \"v1\", \"metadata\": { \"name\": \"$ARGOCD_APP_NAME\", \"namespace\": \"$ARGOCD_APP_NAMESPACE\", \"annotations\": {\"GitAskpass\": \"$GIT_ASKPASS\", \"GitUsername\": \"$GIT_USERNAME\", \"GitPassword\": \"$GIT_PASSWORD\"}}}"`}, + }, + }, + ). + // add the private repo + And(func() { + FailOnErr(RunCli("repo", "add", repoUrl, "--username", "blah", "--password", accessToken)) + }). + Repo(repoUrl). + Path(appPath). + When(). + Create(). + Sync(). + Then(). + Expect(OperationPhaseIs(OperationSucceeded)). + And(func(app *Application) { + output, err := Run("", "kubectl", "-n", DeploymentNamespace(), "get", "cm", Name(), "-o", "jsonpath={.metadata.annotations.GitAskpass}") + assert.NoError(t, err) + assert.Equal(t, "git-ask-pass.sh", output) + }). + And(func(app *Application) { + output, err := Run("", "kubectl", "-n", DeploymentNamespace(), "get", "cm", Name(), "-o", "jsonpath={.metadata.annotations.GitUsername}") + assert.NoError(t, err) + assert.Equal(t, "blah", output) + }). + And(func(app *Application) { + output, err := Run("", "kubectl", "-n", DeploymentNamespace(), "get", "cm", Name(), "-o", "jsonpath={.metadata.annotations.GitPassword}") + assert.NoError(t, err) + assert.Equal(t, accessToken, output) + }) +} diff --git a/test/e2e/fixture/app/actions.go b/test/e2e/fixture/app/actions.go index d0bc10dde6772..93f551e09a65f 100644 --- a/test/e2e/fixture/app/actions.go +++ b/test/e2e/fixture/app/actions.go @@ -47,6 +47,10 @@ func (a *Actions) Create() *Actions { args = append(args, "--nameprefix", a.context.namePrefix) } + if a.context.configManagementPlugin != "" { + args = append(args, "--config-management-plugin", a.context.configManagementPlugin) + } + a.runCli(args...) return a diff --git a/test/e2e/fixture/app/consequences.go b/test/e2e/fixture/app/consequences.go index d7e73407b5035..f9e7b18ff8ba7 100644 --- a/test/e2e/fixture/app/consequences.go +++ b/test/e2e/fixture/app/consequences.go @@ -42,6 +42,10 @@ func (c *Consequences) And(block func(app *Application)) *Consequences { return c } +func (c *Consequences) Given() *Context { + return c.context +} + func (c *Consequences) When() *Actions { return c.actions } diff --git a/test/e2e/fixture/app/context.go b/test/e2e/fixture/app/context.go index a800a2058f58a..095bba25b614d 100644 --- a/test/e2e/fixture/app/context.go +++ b/test/e2e/fixture/app/context.go @@ -4,20 +4,22 @@ import ( "testing" . "github.com/argoproj/argo-cd/common" + "github.com/argoproj/argo-cd/pkg/apis/application/v1alpha1" "github.com/argoproj/argo-cd/test/e2e/fixture" ) // this implements the "given" part of given/when/then type Context struct { - t *testing.T - path string - name string - destServer string - env string - parameters []string - namePrefix string - resource string - prune bool + t *testing.T + path string + name string + destServer string + env string + parameters []string + namePrefix string + resource string + prune bool + configManagementPlugin string } func Given(t *testing.T) *Context { @@ -61,6 +63,18 @@ func (c *Context) NamePrefix(namePrefix string) *Context { return c } +func (c *Context) ResourceOverrides(overrides map[string]v1alpha1.ResourceOverride) *Context { + fixture.SetResourceOverrides(overrides) + return c +} + +// this both configures the plugin, but forces use of it +func (c *Context) ConfigManagementPlugin(plugin v1alpha1.ConfigManagementPlugin) *Context { + fixture.SetConfigManagementPlugin(plugin) + c.configManagementPlugin = plugin.Name + return c +} + func (c *Context) And(block func()) *Context { block() return c diff --git a/test/e2e/fixture/fixture.go b/test/e2e/fixture/fixture.go index abb7bc0c78859..cff9eeccbaf7f 100644 --- a/test/e2e/fixture/fixture.go +++ b/test/e2e/fixture/fixture.go @@ -22,9 +22,11 @@ import ( . "github.com/argoproj/argo-cd/errors" argocdclient "github.com/argoproj/argo-cd/pkg/apiclient" sessionpkg "github.com/argoproj/argo-cd/pkg/apiclient/session" + "github.com/argoproj/argo-cd/pkg/apis/application/v1alpha1" appclientset "github.com/argoproj/argo-cd/pkg/client/clientset/versioned" "github.com/argoproj/argo-cd/util" grpcutil "github.com/argoproj/argo-cd/util/grpc" + "github.com/argoproj/argo-cd/util/rand" "github.com/argoproj/argo-cd/util/settings" ) @@ -40,10 +42,11 @@ const ( var ( id string + name string KubeClientset kubernetes.Interface AppClientset appclientset.Interface ArgoCDClientset argocdclient.Client - SettingsManager *settings.SettingsManager + settingsManager *settings.SettingsManager apiServerAddress string token string plainText bool @@ -93,7 +96,7 @@ func init() { }) CheckError(err) - SettingsManager = settings.NewSettingsManager(context.Background(), KubeClientset, "argocd-e2e") + settingsManager = settings.NewSettingsManager(context.Background(), KubeClientset, "argocd-e2e") token = sessionResponse.Token plainText = !tlsTestResult.TLS @@ -101,11 +104,11 @@ func init() { } func Name() string { - return id + return name } func repoDirectory() string { - return path.Join(tmpDir, id) + return path.Join(tmpDir, name) } func SetRepoURL(url string) { repoUrl = url @@ -116,12 +119,12 @@ func RepoURL() string { } func DeploymentNamespace() string { - return fmt.Sprintf("argocd-e2e-ns-%s", id) + return dnsFriendly(fmt.Sprintf("argocd-e2e-%s", id)) } // creates a secret for the current test, this currently can only create a single secret func CreateSecret(username, password string) string { - secretName := fmt.Sprintf("argocd-e2e-secret-%s", id) + secretName := fmt.Sprintf("argocd-e2e-%s", name) FailOnErr(Run("", "kubectl", "create", "secret", "generic", secretName, "--from-literal=username="+username, "--from-literal=password="+password, @@ -130,6 +133,25 @@ func CreateSecret(username, password string) string { return secretName } +func Settings(consumer func(s *settings.ArgoCDSettings)) { + s, err := settingsManager.GetSettings() + CheckError(err) + consumer(s) + CheckError(settingsManager.SaveSettings(s)) +} + +func SetResourceOverrides(overrides map[string]v1alpha1.ResourceOverride) { + Settings(func(s *settings.ArgoCDSettings) { + s.ResourceOverrides = overrides + }) +} + +func SetConfigManagementPlugin(plugin v1alpha1.ConfigManagementPlugin) { + Settings(func(s *settings.ArgoCDSettings) { + s.ConfigManagementPlugins = []v1alpha1.ConfigManagementPlugin{plugin} + }) +} + func EnsureCleanState(t *testing.T) { start := time.Now() @@ -148,9 +170,9 @@ func EnsureCleanState(t *testing.T) { FailOnErr(Run("", "kubectl", "delete", "ns", "-l", testingLabel+"=true", "--field-selector", "status.phase=Active", "--wait=false")) // reset settings - s, err := SettingsManager.GetSettings() + s, err := settingsManager.GetSettings() CheckError(err) - CheckError(SettingsManager.SaveSettings(&settings.ArgoCDSettings{ + CheckError(settingsManager.SaveSettings(&settings.ArgoCDSettings{ // changing theses causes a restart AdminPasswordHash: s.AdminPasswordHash, AdminPasswordMtime: s.AdminPasswordMtime, @@ -168,8 +190,10 @@ func EnsureCleanState(t *testing.T) { // remove tmp dir CheckError(os.RemoveAll(tmpDir)) - // new random ID - id = dnsFriendly(t.Name()) + // name based on test name + name = dnsFriendly(t.Name()) + // random id - unique across test runs + id = name + "-" + strings.ToLower(rand.RandString(5)) repoUrl = fmt.Sprintf("file://%s", repoDirectory()) // create tmp dir @@ -186,7 +210,7 @@ func EnsureCleanState(t *testing.T) { FailOnErr(Run("", "kubectl", "create", "ns", DeploymentNamespace())) FailOnErr(Run("", "kubectl", "label", "ns", DeploymentNamespace(), testingLabel+"=true")) - log.WithFields(log.Fields{"duration": time.Since(start), "id": id}).Info("clean state") + log.WithFields(log.Fields{"duration": time.Since(start), "name": name, "id": id}).Info("clean state") } func RunCli(args ...string) (string, error) { diff --git a/test/e2e/fixture/util.go b/test/e2e/fixture/util.go index 443692b6b4c90..fae2af2e6aa86 100644 --- a/test/e2e/fixture/util.go +++ b/test/e2e/fixture/util.go @@ -9,7 +9,12 @@ var matchFirstCap = regexp.MustCompile("(.)([A-Z][a-z]+)") var matchAllCap = regexp.MustCompile("([a-z0-9])([A-Z])") func dnsFriendly(str string) string { - snake := matchFirstCap.ReplaceAllString(str, "${1}-${2}") - snake = matchAllCap.ReplaceAllString(snake, "${1}-${2}") - return strings.ToLower(snake) + str = matchFirstCap.ReplaceAllString(str, "${1}-${2}") + str = matchAllCap.ReplaceAllString(str, "${1}-${2}") + str = strings.ToLower(str) + // DNS names must be <=63 chars + if len(str) > 63 { + str = str[:63] + } + return str } diff --git a/util/git/creds.go b/util/git/creds.go new file mode 100644 index 0000000000000..b74de856c5358 --- /dev/null +++ b/util/git/creds.go @@ -0,0 +1,6 @@ +package git + +// client credentials for Git HTTPS connection +type Creds struct { + Username, Password string +} diff --git a/util/kustomize/kustomize.go b/util/kustomize/kustomize.go index 38e585df1eae9..d1f4ffca407d6 100644 --- a/util/kustomize/kustomize.go +++ b/util/kustomize/kustomize.go @@ -16,6 +16,7 @@ import ( "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "github.com/argoproj/argo-cd/pkg/apis/application/v1alpha1" + "github.com/argoproj/argo-cd/util/git" "github.com/argoproj/argo-cd/util/kube" ) @@ -42,13 +43,8 @@ type Kustomize interface { Build(opts *v1alpha1.ApplicationSourceKustomize) ([]*unstructured.Unstructured, []ImageTag, []Image, error) } -type GitCredentials struct { - Username string - Password string -} - // NewKustomizeApp create a new wrapper to run commands on the `kustomize` command-line tool. -func NewKustomizeApp(path string, creds *GitCredentials) Kustomize { +func NewKustomizeApp(path string, creds *git.Creds) Kustomize { return &kustomize{ path: path, creds: creds, @@ -57,7 +53,7 @@ func NewKustomizeApp(path string, creds *GitCredentials) Kustomize { type kustomize struct { path string - creds *GitCredentials + creds *git.Creds } func (k *kustomize) Build(opts *v1alpha1.ApplicationSourceKustomize) ([]*unstructured.Unstructured, []ImageTag, []Image, error) { diff --git a/util/kustomize/kustomize_test.go b/util/kustomize/kustomize_test.go index 1266957ce8213..e76c596dec94a 100644 --- a/util/kustomize/kustomize_test.go +++ b/util/kustomize/kustomize_test.go @@ -12,6 +12,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/argoproj/argo-cd/pkg/apis/application/v1alpha1" + "github.com/argoproj/argo-cd/util/git" ) // TODO: move this into shared test package after resolving import cycle @@ -141,7 +142,7 @@ func TestPrivateRemoteBase(t *testing.T) { assert.NoError(t, err) defer func() { _ = os.Setenv("PATH", osPath) }() - kust := NewKustomizeApp("./testdata/private-remote-base", &GitCredentials{Username: PrivateGitUsername, Password: PrivateGitPassword}) + kust := NewKustomizeApp("./testdata/private-remote-base", &git.Creds{Username: PrivateGitUsername, Password: PrivateGitPassword}) objs, _, _, err := kust.Build(nil) assert.NoError(t, err) diff --git a/util/settings/settings.go b/util/settings/settings.go index 980c2a48ac328..3e36da8200457 100644 --- a/util/settings/settings.go +++ b/util/settings/settings.go @@ -575,6 +575,16 @@ func (mgr *SettingsManager) SaveSettings(settings *ArgoCDSettings) error { delete(argoCDCM.Data, resourceExclusionsKey) } + if len(settings.ConfigManagementPlugins) > 0 { + yamlBytes, err := yaml.Marshal(settings.ConfigManagementPlugins) + if err != nil { + return err + } + argoCDCM.Data[configManagementPluginsKey] = string(yamlBytes) + } else { + delete(argoCDCM.Data, configManagementPluginsKey) + } + if createCM { _, err = mgr.clientset.CoreV1().ConfigMaps(mgr.namespace).Create(argoCDCM) } else {