diff --git a/Dockerfile b/Dockerfile index f30b9c1252e95..1a3508a40b270 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,4 +1,4 @@ -ARG BASE_IMAGE=docker.io/library/ubuntu:21.10 +ARG BASE_IMAGE=docker.io/library/ubuntu:22.04 #################################################################################################### # Builder image # Initial stage which pulls prepares build dependencies and CLI tooling we need for our final image @@ -127,4 +127,4 @@ RUN ln -s /usr/local/bin/argocd /usr/local/bin/argocd-server && \ ln -s /usr/local/bin/argocd /usr/local/bin/argocd-applicationset-controller && \ ln -s /usr/local/bin/argocd /usr/local/bin/argocd-k8s-auth -USER 999 \ No newline at end of file +USER 999 diff --git a/USERS.md b/USERS.md index 4c7af0307ca97..d2db5516c3144 100644 --- a/USERS.md +++ b/USERS.md @@ -72,6 +72,7 @@ Currently, the following organizations are **officially** using Argo CD: 1. [GMETRI](https://gmetri.com/) 1. [Gojek](https://www.gojek.io/) 1. [Greenpass](https://www.greenpass.com.br/) +1. [Grupo MasMovil](https://grupomasmovil.com/en/) 1. [Handelsbanken](https://www.handelsbanken.se) 1. [Healy](https://www.healyworld.net) 1. [Helio](https://helio.exchange) diff --git a/cmd/argocd/commands/admin/settings_rbac.go b/cmd/argocd/commands/admin/settings_rbac.go index 164acf7a53eed..c66e1ba50bfa8 100644 --- a/cmd/argocd/commands/admin/settings_rbac.go +++ b/cmd/argocd/commands/admin/settings_rbac.go @@ -158,7 +158,7 @@ argocd admin settings rbac can someuser create application 'default/app' --defau log.Fatalf("could not create k8s client: %v", err) } - userPolicy, newDefaultRole = getPolicy(policyFile, realClientset, namespace) + userPolicy, newDefaultRole, matchMode := getPolicy(policyFile, realClientset, namespace) // Use built-in policy as augmentation if requested if useBuiltin { @@ -171,7 +171,7 @@ argocd admin settings rbac can someuser create application 'default/app' --defau defaultRole = newDefaultRole } - res := checkPolicy(subject, action, resource, subResource, builtinPolicy, userPolicy, defaultRole, strict) + res := checkPolicy(subject, action, resource, subResource, builtinPolicy, userPolicy, defaultRole, matchMode, strict) if res { if !quiet { fmt.Println("Yes") @@ -213,7 +213,7 @@ a local file, and in either CSV or K8s ConfigMap format. c.HelpFunc()(c, args) log.Fatalf("Please specify policy to validate using --policy-file") } - userPolicy, _ := getPolicy(policyFile, nil, "") + userPolicy, _, _ := getPolicy(policyFile, nil, "") if userPolicy != "" { if err := rbac.ValidatePolicy(userPolicy); err == nil { fmt.Printf("Policy is valid.\n") @@ -232,11 +232,11 @@ a local file, and in either CSV or K8s ConfigMap format. // Load user policy file if requested or use Kubernetes client to get the // appropriate ConfigMap from the current context -func getPolicy(policyFile string, kubeClient kubernetes.Interface, namespace string) (userPolicy string, defaultRole string) { +func getPolicy(policyFile string, kubeClient kubernetes.Interface, namespace string) (userPolicy string, defaultRole string, matchMode string) { var err error if policyFile != "" { // load from file - userPolicy, defaultRole, err = getPolicyFromFile(policyFile) + userPolicy, defaultRole, matchMode, err = getPolicyFromFile(policyFile) if err != nil { log.Fatalf("could not read policy file: %v", err) } @@ -245,23 +245,24 @@ func getPolicy(policyFile string, kubeClient kubernetes.Interface, namespace str if err != nil { log.Fatalf("could not get configmap: %v", err) } - userPolicy, defaultRole = getPolicyFromConfigMap(cm) + userPolicy, defaultRole, matchMode = getPolicyFromConfigMap(cm) } - return userPolicy, defaultRole + return userPolicy, defaultRole, matchMode } // getPolicyFromFile loads a RBAC policy from given path -func getPolicyFromFile(policyFile string) (string, string, error) { +func getPolicyFromFile(policyFile string) (string, string, string, error) { var ( userPolicy string defaultRole string + matchMode string ) upol, err := ioutil.ReadFile(policyFile) if err != nil { log.Fatalf("error opening policy file: %v", err) - return "", "", err + return "", "", "", err } // Try to unmarshal the input file as ConfigMap first. If it succeeds, we @@ -271,14 +272,14 @@ func getPolicyFromFile(policyFile string) (string, string, error) { if err != nil { userPolicy = string(upol) } else { - userPolicy, defaultRole = getPolicyFromConfigMap(upolCM) + userPolicy, defaultRole, matchMode = getPolicyFromConfigMap(upolCM) } - return userPolicy, defaultRole, nil + return userPolicy, defaultRole, matchMode, nil } // Retrieve policy information from a ConfigMap -func getPolicyFromConfigMap(cm *corev1.ConfigMap) (string, string) { +func getPolicyFromConfigMap(cm *corev1.ConfigMap) (string, string, string) { var ( userPolicy string defaultRole string @@ -295,7 +296,7 @@ func getPolicyFromConfigMap(cm *corev1.ConfigMap) (string, string) { } } - return userPolicy, defaultRole + return userPolicy, defaultRole, cm.Data[rbac.ConfigMapMatchModeKey] } // getPolicyConfigMap fetches the RBAC config map from K8s cluster @@ -309,9 +310,10 @@ func getPolicyConfigMap(client kubernetes.Interface, namespace string) (*corev1. // checkPolicy checks whether given subject is allowed to execute specified // action against specified resource -func checkPolicy(subject, action, resource, subResource, builtinPolicy, userPolicy, defaultRole string, strict bool) bool { +func checkPolicy(subject, action, resource, subResource, builtinPolicy, userPolicy, defaultRole, matchMode string, strict bool) bool { enf := rbac.NewEnforcer(nil, "argocd", "argocd-rbac-cm", nil) enf.SetDefaultRole(defaultRole) + enf.SetMatchMode(matchMode) if builtinPolicy != "" { if err := enf.SetBuiltinPolicy(builtinPolicy); err != nil { log.Fatalf("could not set built-in policy: %v", err) diff --git a/cmd/argocd/commands/admin/settings_rbac_test.go b/cmd/argocd/commands/admin/settings_rbac_test.go index 0574f409e4c25..ae58c7a34bc5e 100644 --- a/cmd/argocd/commands/admin/settings_rbac_test.go +++ b/cmd/argocd/commands/admin/settings_rbac_test.go @@ -40,15 +40,17 @@ func Test_isValidRBACResource(t *testing.T) { } func Test_PolicyFromCSV(t *testing.T) { - uPol, dRole := getPolicy("testdata/rbac/policy.csv", nil, "") + uPol, dRole, matchMode := getPolicy("testdata/rbac/policy.csv", nil, "") require.NotEmpty(t, uPol) require.Empty(t, dRole) + require.Empty(t, matchMode) } func Test_PolicyFromYAML(t *testing.T) { - uPol, dRole := getPolicy("testdata/rbac/argocd-rbac-cm.yaml", nil, "") + uPol, dRole, matchMode := getPolicy("testdata/rbac/argocd-rbac-cm.yaml", nil, "") require.NotEmpty(t, uPol) require.Equal(t, "role:unknown", dRole) + require.Empty(t, matchMode) } func Test_PolicyFromK8s(t *testing.T) { @@ -64,28 +66,86 @@ func Test_PolicyFromK8s(t *testing.T) { "policy.default": "role:unknown", }, }) - uPol, dRole := getPolicy("", kubeclientset, "argocd") + uPol, dRole, matchMode := getPolicy("", kubeclientset, "argocd") require.NotEmpty(t, uPol) require.Equal(t, "role:unknown", dRole) + require.Equal(t, "", matchMode) t.Run("get applications", func(t *testing.T) { - ok := checkPolicy("role:user", "get", "applications", "*/*", assets.BuiltinPolicyCSV, uPol, dRole, true) + ok := checkPolicy("role:user", "get", "applications", "*/*", assets.BuiltinPolicyCSV, uPol, dRole, "", true) require.True(t, ok) }) t.Run("get clusters", func(t *testing.T) { - ok := checkPolicy("role:user", "get", "clusters", "*", assets.BuiltinPolicyCSV, uPol, dRole, true) + ok := checkPolicy("role:user", "get", "clusters", "*", assets.BuiltinPolicyCSV, uPol, dRole, "", true) require.True(t, ok) }) t.Run("get certificates", func(t *testing.T) { - ok := checkPolicy("role:user", "get", "certificates", "*", assets.BuiltinPolicyCSV, uPol, dRole, true) + ok := checkPolicy("role:user", "get", "certificates", "*", assets.BuiltinPolicyCSV, uPol, dRole, "", true) require.False(t, ok) }) t.Run("get certificates by default role", func(t *testing.T) { - ok := checkPolicy("role:user", "get", "certificates", "*", assets.BuiltinPolicyCSV, uPol, "role:readonly", true) + ok := checkPolicy("role:user", "get", "certificates", "*", assets.BuiltinPolicyCSV, uPol, "role:readonly", "glob", true) require.True(t, ok) }) t.Run("get certificates by default role without builtin policy", func(t *testing.T) { - ok := checkPolicy("role:user", "get", "certificates", "*", "", uPol, "role:readonly", true) + ok := checkPolicy("role:user", "get", "certificates", "*", "", uPol, "role:readonly", "glob", true) + require.False(t, ok) + }) + t.Run("use regex match mode instead of glob", func(t *testing.T) { + ok := checkPolicy("role:user", "get", "certificates", ".*", assets.BuiltinPolicyCSV, uPol, "role:readonly", "regex", true) + require.False(t, ok) + }) +} + +func Test_PolicyFromK8sUsingRegex(t *testing.T) { + policy := ` +p, role:user, clusters, get, .+, allow +p, role:user, clusters, get, https://kubernetes.*, deny +p, role:user, applications, get, .*, allow +p, role:user, applications, create, .*/.*, allow` + + kubeclientset := fake.NewSimpleClientset(&v1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "argocd-rbac-cm", + Namespace: "argocd", + }, + Data: map[string]string{ + "policy.csv": policy, + "policy.default": "role:unknown", + "policy.matchMode": "regex", + }, + }) + uPol, dRole, matchMode := getPolicy("", kubeclientset, "argocd") + require.NotEmpty(t, uPol) + require.Equal(t, "role:unknown", dRole) + require.Equal(t, "regex", matchMode) + + builtInPolicy := ` +p, role:readonly, certificates, get, .*, allow +p, role:, certificates, get, .*, allow` + + t.Run("get applications", func(t *testing.T) { + ok := checkPolicy("role:user", "get", "applications", ".*/.*", builtInPolicy, uPol, dRole, "regex", true) + require.True(t, ok) + }) + t.Run("get clusters", func(t *testing.T) { + ok := checkPolicy("role:user", "get", "clusters", ".*", builtInPolicy, uPol, dRole, "regex", true) + require.True(t, ok) + }) + t.Run("get certificates", func(t *testing.T) { + ok := checkPolicy("role:user", "get", "certificates", ".*", builtInPolicy, uPol, dRole, "regex", true) + require.False(t, ok) + }) + t.Run("get certificates by default role", func(t *testing.T) { + ok := checkPolicy("role:user", "get", "certificates", ".*", builtInPolicy, uPol, "role:readonly", "regex", true) + require.True(t, ok) + }) + t.Run("get certificates by default role without builtin policy", func(t *testing.T) { + ok := checkPolicy("role:user", "get", "certificates", ".*", "", uPol, "role:readonly", "regex", true) + require.False(t, ok) + }) + t.Run("use glob match mode instead of regex", func(t *testing.T) { + ok := checkPolicy("role:user", "get", "certificates", ".+", builtInPolicy, uPol, dRole, "glob", true) require.False(t, ok) }) } diff --git a/docs/operator-manual/rbac.md b/docs/operator-manual/rbac.md index a43d446c6a1e4..da4c9d5977871 100644 --- a/docs/operator-manual/rbac.md +++ b/docs/operator-manual/rbac.md @@ -30,7 +30,29 @@ Breaking down the permissions definition differs slightly between applications a Resources: `clusters`, `projects`, `applications`, `repositories`, `certificates`, `accounts`, `gpgkeys`, `logs`, `exec` -Actions: `get`, `create`, `update`, `delete`, `sync`, `override`, `action` +Actions: `get`, `create`, `update`, `delete`, `sync`, `override`, +`action/` + +#### Application resources + +The resource path for application objects is of the form +`/`. + +Delete access to sub-resources of a project, such as a rollout or a pod, cannot +be managed granularly. `/` grants access to all +subresources of an application. + +#### The `action` action + +The `action` action corresponds to either built-in resource customizations defined +[in the Argo CD repository](https://github.com/argoproj/argo-cd/search?q=filename%3Aaction.lua+path%3Aresource_customizations), +or to [custom resource actions](resource_actions.md#custom-resource-actions) defined by you. +The `action` path is of the form `action///`. For +example, a resource customization path +`resource_customizations/extensions/DaemonSet/actions/restart/action.lua` +corresponds to the `action` path `action/extensions/DaemonSet/restart`. You can +also use glob patterns in the action path: `action/*` (or regex patterns if you have +[enabled the `regex` match mode](https://github.com/argoproj/argo-cd/blob/master/docs/operator-manual/argocd-rbac-cm.yaml)). #### `exec` resource diff --git a/docs/operator-manual/upgrading/2.3-2.4.md b/docs/operator-manual/upgrading/2.3-2.4.md index 52714cffd8b2e..be7c486ef1590 100644 --- a/docs/operator-manual/upgrading/2.3-2.4.md +++ b/docs/operator-manual/upgrading/2.3-2.4.md @@ -2,14 +2,13 @@ ## KSonnet support is removed -The [https://ksonnet.io/] had been deprecated in [2019](https://github.com/ksonnet/ksonnet/pull/914/files) and no longer maintained. -The time has come to remove it from the ArgoCD. +Ksonnet was deprecated in [2019](https://github.com/ksonnet/ksonnet/pull/914/files) and is no longer maintained. +The time has come to remove it from the Argo CD. ## Helm 2 support is removed -Helm 2 is not been officially supported since [Nov 2020](https://helm.sh/blog/helm-2-becomes-unsupported/). In order to ensure a smooth transition -Helm 2 support was preserved in the Argo CD. We feel that Helm 3 is stable and it is time to drop Helm 2 support. - +Helm 2 has not been officially supported since [Nov 2020](https://helm.sh/blog/helm-2-becomes-unsupported/). In order to ensure a smooth transition, +Helm 2 support was preserved in the Argo CD. We feel that Helm 3 is stable, and it is time to drop Helm 2 support. ## Configure RBAC to account for new `exec` resource @@ -77,11 +76,20 @@ p, role:test-db-admins, applications, *, staging-db-admins/*, allow p, role:test-db-admins, logs, get, staging-db-admins/*, allow ``` -### Known UI issue +## Known UI issue Currently, upon pressing the "LOGS" tab in pod view by users who don't have an explicit allow get logs policy, the red "unable to load data: Internal error" is received in the bottom of the screen, and "Failed to load data, please try again" is displayed. -## Remove the shared volume from any sidecar plugins +## Test repo-server with its new dedicated Service Account + +As a security enhancement, the argocd-repo-server Deployment uses its own Service Account instead of `default`. + +If you have a custom environment that might depend on repo-server using the `default` Service Account (such as a plugin +that uses the Service Account for auth), be sure to test before deploying the 2.4 upgrade to production. + +## Plugins + +### Remove the shared volume from any sidecar plugins As a security enhancement, [sidecar plugins](../../user-guide/config-management-plugins.md#option-2-configure-plugin-via-sidecar) no longer share the /tmp directory with the repo-server. @@ -108,9 +116,39 @@ spec: emptyDir: {} ``` -## Test repo-server with its new dedicated Service Account +### Update plugins to use newly-prefixed environment variables -As a security enhancement, the argocd-repo-server Deployment uses its own Service Account instead of `default`. +If you use plugins that depend on user-supplied environment variables, then they must be updated to be compatible with +Argo CD 2.4. Here is an example of user-supplied environment variables in the `plugin` section of an Application spec: -If you have a custom environment that might depend on repo-server using the `default` Service Account (such as a plugin -that uses the Service Account for auth), be sure to test before deploying the 2.4 upgrade to production. +```yaml +apiVersion: argoproj.io/v1alpha1 +kind: Application +spec: + source: + plugin: + env: + - name: FOO + value: bar +``` + +Going forward, all user-supplied environment variables will be prefixed with `ARGOCD_ENV_` before being sent to the +plugin's `init`, `generate`, or `discover` commands. This prevents users from setting potentially-sensitive environment +variables. + +If you have written a custom plugin which handles user-provided environment variables, update it to handle the new +prefix. + +If you use a third-party plugin which does not explicitly advertise Argo CD 2.4 support, it might not handle the +prefixed environment variables. Open an issue with the plugin's authors and confirm support before upgrading to Argo CD +2.4. + +### Confirm sidecar plugins have all necessary environment variables + +A bug in < 2.4 caused `init` and `generate` commands to receive environment variables from the main repo-server +container, taking precedence over environment variables from the plugin's sidecar. + +Starting in 2.4, sidecar plugins will not receive environment variables from the main repo-server container. Make sure +that any environment variables necessary for the sidecar plugin to function are set on the sidecar plugin. + +argocd-cm plugins will continue to receive environment variables from the main repo-server container. diff --git a/docs/operator-manual/user-management/google.md b/docs/operator-manual/user-management/google.md index e4e26cd180584..308cf023bd4cd 100644 --- a/docs/operator-manual/user-management/google.md +++ b/docs/operator-manual/user-management/google.md @@ -50,7 +50,7 @@ If you've never configured this, you'll be redirected straight to this if you tr ### Configure Argo to use OpenID Connect -Edit `argo-cm` and add the following `dex.config` to the data section, replacing `clientID` and `clientSecret` with the values you saved before: +Edit `argocd-cm` and add the following `dex.config` to the data section, replacing `clientID` and `clientSecret` with the values you saved before: ```yaml data: @@ -113,7 +113,7 @@ data: ### Configure Argo to use the new Google SAML App -Edit `argo-cm` and add the following `dex.config` to the data section, replacing the `caData`, `argocd.example.com`, `sso-url`, and optionally `google-entity-id` with your values from the Google SAML App: +Edit `argocd-cm` and add the following `dex.config` to the data section, replacing the `caData`, `argocd.example.com`, `sso-url`, and optionally `google-entity-id` with your values from the Google SAML App: ```yaml data: @@ -211,7 +211,7 @@ Go through the same steps as in [OpenID Connect using Dex](#openid-connect-using defaultMode: 420 secretName: argocd-google-groups-json -3. Edit `argo-cm` and add the following `dex.config` to the data section, replacing `clientID` and `clientSecret` with the values you saved before, `adminEmail` with the address for the admin user you're going to impersonate, and editing `redirectURI` with your Argo CD domain: +3. Edit `argocd-cm` and add the following `dex.config` to the data section, replacing `clientID` and `clientSecret` with the values you saved before, `adminEmail` with the address for the admin user you're going to impersonate, and editing `redirectURI` with your Argo CD domain: dex.config: | connectors: diff --git a/docs/proposals/server-side-apply.md b/docs/proposals/server-side-apply.md new file mode 100644 index 0000000000000..1fa6c98e313d5 --- /dev/null +++ b/docs/proposals/server-side-apply.md @@ -0,0 +1,239 @@ +--- +title: Server-Side Apply + +authors: +- "@leoluz" + +sponsors: +- TBD + +reviewers: +- TBD + +approvers: +- TBD + +creation-date: 2022-03-17 + +--- + +# Server-Side Apply support for ArgoCD + +[Server-Side Apply (SSA)][1] allows calculating the final patch to update +resources in Kubernetes in the server instead of the client. This proposal +describes how ArgoCD can leverage SSA during syncs. + +* [Open Questions](#open-questions) + * [[Q-1] How to handle conflicts?](#q-1-how-to-handle-conflicts) + * [[Q-2] Should we support multiple managers?](#q-2-should-we-support-multiple-managers) +* [Summary](#summary) +* [Motivation](#motivation) + * [Better interoperability with Admission Controllers](#better-interoperability-with-admission-controllers) + * [Better resource conflict management](#better-resource-conflict-management) + * [Better CRD support](#better-crd-support) +* [Goals](#goals) + * [[G-1] Fine grained configuration](#g-1-fine-grained-configuration) + * [[G-2] Strategic merge patch while diffing](#g-2-strategic-merge-patch-while-diffing) + * [[G-3] Admission Controllers compatibility](#g-3-admission-controllers-compatibility) + * [[G-4] Conflict management](#g-4-conflict-management) + * [[G-5] Register a proper manager](#g-5-register-a-proper-manager) +* [Non-Goals](#non-goals) +* [Proposal](#proposal) + * [Non-Functional Requirements](#non-functional-requirements) + * [Use cases](#use-cases) + * [[UC-1]: enable SSA at the controller level](#uc-1-as-a-user-i-would-like-enable-ssa-at-the-controller-level-so-all-application-are-applied-server-side) + * [[UC-2]: enable SSA at the Application level](#uc-2-as-a-user-i-would-like-enable-ssa-at-the-application-level-so-all-resources-are-applied-server-side) + * [[UC-3]: enable SSA at the resource level](#uc-3-as-a-user-i-would-like-enable-ssa-at-the-resource-level-so-only-a-single-manifest-is-applied-server-side) + * [Security Considerations](#security-considerations) + * [Risks and Mitigations](#risks-and-mitigations) + * [[R-1] Supported K8s version check](#r-1-supported-k8s-version-check) + * [[R-2] Alternating Server-Side Client-Side syncs](#r-2-alternating-server-side-client-side-syncs) + * [Upgrade / Downgrade](#upgrade--downgrade) +* [Drawbacks](#drawbacks) + +--- + +## Open Questions + +### [Q-1] How to handle conflicts? +When SSA is enabled, the server may return field conflicts with other managers. +What ArgoCD controller should do in case of conflict? Just force the sync and +log warnings (like some other controllers do?) + +#### Conclusion +The first version should use the force flag and override even if there are +conflicts. We could improve and add other options once there is a use case. + +### [Q-2] Should we support multiple managers? +Should Server-Side Apply support in ArgoCD be implemented allowing multiple +managers for the same controller? ([more details][10]) + +## Summary + +ArgoCD can benefit from [Server-Side Apply][1] during syncs. A few +improvements to consider: + +- More reliable dry-runs (as admission controller is executed) ([ISSUE-804][5]) +- [Syncs always run][2] mutating webhooks (even without diff) +- [Fix big CRD][3] sync issues +- Better interoperability with different controllers + +Kubernetes SSA Proposal ([KEP-555][13]) has more details about how it works. + +## Motivation + +ArgoCD uses kubectl library while syncing resources in the cluster. Kubectl uses +by default a 3-way-merge logic between the live state (in k8s), desired state +(in git) and the previous state (`last-applied-configuration` annotation) to +calculate diffs and patch resources in the cluster. This logic is executed in +the client (ArgoCD) and once the patch is calculated it is then sent to the +server. + +This strategy works well in the majority of the use cases. However, there are +some scenarios where calculating patches in the client side can cause problems. + +Some of the known problems about using client-side approach: + +### Better interoperability with Admission Controllers + +More and more users are adopting and configuring [Admission Controllers][4] in +Kubernetes with different flavors of Validating Webhooks and Mutating Webhooks. +Admission Controllers will only execute in server-side. In cases where users +rely on dry-run executions to decide if they should proceed with a deployment, +having the patch calculated at the client side might provide undesired results. +Having SSA enabled syncs also guaranties that Admission Controllers are always +executed, even when there is no diff in the resource. + +### Better resource conflict management + +Server-Side Apply will better handle and identify conflicts during syncs by +analyzing the `managedFields` metadata available in all Kubernetes resources +(since kubernetes 1.18). + +### Better CRD support + +By not having to rely on the `last-applied-configuration` annotation, SSA would +help with failing syncs caused by exceeded annotation size limit. This is a +common issue when syncing CRDs with large schemas. + +## Goals + +All following goals should be achieve in order to conclude this proposal: + +#### [G-1] Fine grained configuration + +- Provide the ability for users to define if they want to use SSA during syncs +- Users should be able to enable SSA at the controller level (via binary flag) +(see [UC-1](#uc-1-as-a-user-i-would-like-enable-ssa-at-the-controller-level-so-all-application-are-applied-server-side)) +- Users should be able to enable SSA for a given Application (via syncOptions) +(see [UC-2](#uc-2-as-a-user-i-would-like-enable-ssa-at-the-application-level-so-all-resources-are-applied-server-side)) +- Users should be able to enable SSA at resource level (via annotation) (see +[UC-3](#uc-3-as-a-user-i-would-like-enable-ssa-at-the-resource-level-so-only-a-single-manifest-is-applied-server-side) +- Relates to [ISSUE-2267][6] + +#### [G-2] Strategic merge patch while diffing + +- Diffing needs to support strategic merge patch (see [ISSUE-2268][7]) +- Make sure Services can be patched correctly ([more details][14]) + +#### [G-3] Admission Controllers compatibility + +- Allow Admission Controllers to execute even when there is no diff for a + particular resource. (Needs investigation) ([more details][2]) + +#### [G-4] Conflict management + +- ArgoCD should respect field ownership and provide a configuration to allow + users to define the behavior in case of conflicts (see + [Q-1](#q-1-how-to-handle-conflicts) outcome) + +#### [G-5] Register a proper manager + +- ArgoCD must register itself with a pre-defined manager (suggestion: + `argocd-controller`). It shouldn't rely on the default value defined in the + kubectl code. ([more details][11]) + +## Non-Goals + +TBD + +## Proposal + +Change ArgoCD controller to accept new parameter to enable Server-Side Apply +during syncs. Changes are necessary in ArgoCD as well as in +gitops-engine library. + +### Use cases + +The following use cases should be implemented: + +#### [UC-1]: As a user, I would like enable SSA at the controller level so all Application are applied server-side + +Implement a binary flag to configure ArgoCD to run all syncs using SSA. +(suggestion: `--server-side-apply=true`). Default value should be `false`. + +#### [UC-2]: As a user, I would like enable SSA at the Application level so all resources are applied server-side + +Implement a new syncOption to allow users to enable SSA at the application +level (Suggestion `ServerSideApply=true`). UI needs to be updated to support +this new sync option. If not informed, the controller should keep the current +behaviour (client-side). + +#### [UC-3]: As a user, I would like enable SSA at the resource level so only a single manifest is applied server-side + +Leverage the existing `argocd.argoproj.io/sync-options` annotation allowing the +`ServerSideApply=true` to be informed at the resource level. Must not impact +other sync-options informed in the annotation (make sure this annotation +supports providing multiple options). + +### Security Considerations + +TBD + +### Risks and Mitigations + +#### [R-1] Supported K8s version check + +ArgoCD must check if the target Kubernetes cluster has full support for SSA. The +feature turned [GA in Kubernetes 1.22][8]. Full support for managed fields was +introduced as [beta in Kubernetes 1.18][9]. The implementation must check that +the target kubernetes cluster is running at least version 1.18. If SSA is +enabled and target cluster version < 1.18 ArgoCD should log warning and fallback +to client sync. + +#### [R-2] Alternating Server-Side Client-Side syncs + +Kubernetes SSA proposal ([KEP-555][13]) mentions about alternating between +server-side and client-side applies in the [Upgrade/Downgrade Strategy][12] +section. It is stated that Kubernetes will verify the incoming apply request +validating if the user-agent is `kubectl` to decide if the +`last-applied-configuration` annotation should be updated. ArgoCD relies on this +annotation and the implementation must make sure that this agent is correctly +informed when changing to server-side apply and specifying a manager different +than `kubectl`. This is mainly to make sure that +[G-5](#g-5-register-a-proper-manager) isn't impacting the +client-side/server-side compatibility. + +### Upgrade / Downgrade + +No CRD update necessary as `syncOption` field in Application resource is non-typed +(string array). Upgrade will only require ArgoCD controller update. + +## Drawbacks + +Slight increase in ArgoCD code base complexity. + +[1]: https://kubernetes.io/docs/reference/using-api/server-side-apply/ +[2]: https://github.com/argoproj/argo-cd/issues/2267#issuecomment-920445236 +[3]: https://github.com/prometheus-community/helm-charts/issues/1500#issuecomment-1017961377 +[4]: https://kubernetes.io/docs/reference/access-authn-authz/admission-controllers/ +[5]: https://github.com/argoproj/argo-cd/issues/804 +[6]: https://github.com/argoproj/argo-cd/issues/2267 +[7]: https://github.com/argoproj/argo-cd/issues/2268 +[8]: https://kubernetes.io/blog/2021/08/06/server-side-apply-ga/ +[9]: https://kubernetes.io/blog/2020/04/01/kubernetes-1.18-feature-server-side-apply-beta-2/ +[10]: https://github.com/argoproj/gitops-engine/pull/363#issuecomment-1013641708 +[11]: https://github.com/argoproj/gitops-engine/pull/363#issuecomment-1013289982 +[12]: https://github.com/kubernetes/enhancements/blob/master/keps/sig-api-machinery/555-server-side-apply/README.md#upgrade--downgrade-strategy +[13]: https://github.com/kubernetes/enhancements/blob/master/keps/sig-api-machinery/555-server-side-apply/README.md +[14]: https://github.com/argoproj/argo-cd/pull/8812#discussion_r849140565 diff --git a/docs/user-guide/config-management-plugins.md b/docs/user-guide/config-management-plugins.md index a235a0e7bb6de..06d4cbc64a9e2 100644 --- a/docs/user-guide/config-management-plugins.md +++ b/docs/user-guide/config-management-plugins.md @@ -67,8 +67,6 @@ spec: command: [sh, -c, 'echo "{\"kind\": \"ConfigMap\", \"apiVersion\": \"v1\", \"metadata\": { \"name\": \"$ARGOCD_APP_NAME\", \"namespace\": \"$ARGOCD_APP_NAMESPACE\", \"annotations\": {\"Foo\": \"$FOO\", \"KubeVersion\": \"$KUBE_VERSION\", \"KubeApiVersion\": \"$KUBE_API_VERSIONS\",\"Bar\": \"baz\"}}}"'] discover: fileName: "./subdir/s*.yaml" - allowConcurrency: true - lockRepo: false ``` !!! note @@ -90,9 +88,6 @@ If `discover.fileName` is not provided, the `discover.find.command` is executed application repository is supported by the plugin or not. The `find` command should return a non-error exit code and produce output to stdout when the application source type is supported. -If your plugin makes use of `git` (e.g. `git crypt`), it is advised to set `lockRepo` to `true` so that your plugin will have exclusive access to the -repository at the time it is executed. Otherwise, two applications synced at the same time may result in a race condition and sync failure. - #### 2. Place the plugin configuration file in the sidecar Argo CD expects the plugin configuration file to be located at `/home/argocd/cmp-server/config/plugin.yaml` in the sidecar. @@ -119,8 +114,6 @@ data: command: [sh, -c, 'echo "{\"kind\": \"ConfigMap\", \"apiVersion\": \"v1\", \"metadata\": { \"name\": \"$ARGOCD_APP_NAME\", \"namespace\": \"$ARGOCD_APP_NAMESPACE\", \"annotations\": {\"Foo\": \"$FOO\", \"KubeVersion\": \"$KUBE_VERSION\", \"KubeApiVersion\": \"$KUBE_API_VERSIONS\",\"Bar\": \"baz\"}}}"'] discover: fileName: "./subdir/s*.yaml" - allowConcurrency: true - lockRepo: false ``` #### 3. Register the plugin sidecar @@ -166,7 +159,7 @@ volumes: CMP commands have access to -1. The system environment variables +1. The system environment variables (of the repo-server container for argocd-cm plugins or of the sidecar for sidecar plugins) 2. [Standard build environment](build-environment.md) 3. Variables in the application spec (References to system and build variables will get interpolated in the variables' values): @@ -183,6 +176,19 @@ spec: value: test-$ARGOCD_APP_REVISION ``` +!!! note + The `discover.command` command only has access to the above environment starting with v2.4. + +> v2.4 + +Before reaching the `init.command`, `generate.command`, and `discover.command` commands, Argo CD prefixes all +user-supplied environment variables (#3 above) with `ARGOCD_ENV_`. This prevents users from directly setting +potentially-sensitive environment variables. + +If your plugin was written before 2.4 and depends on user-supplied environment variables, then you will need to update +your plugin's behavior to work with 2.4. If you use a third-party plugin, make sure they explicitly advertise support +for 2.4. + ## Using a CMP If your CMP is defined in the `argocd-cm` ConfigMap, you can create a new Application using the CLI. Replace diff --git a/go.mod b/go.mod index 8082162d706ac..1f6222f5ade5a 100644 --- a/go.mod +++ b/go.mod @@ -75,7 +75,7 @@ require ( github.com/whilp/git-urls v0.0.0-20191001220047-6db9661140c0 github.com/xanzy/go-gitlab v0.60.0 github.com/yuin/gopher-lua v0.0.0-20200816102855-ee81675732da - golang.org/x/crypto v0.0.0-20210817164053-32db794688a5 + golang.org/x/crypto v0.0.0-20220525230936-793ad666bf5e golang.org/x/net v0.0.0-20211209124913-491a49abca63 golang.org/x/oauth2 v0.0.0-20211104180415-d3ed0bb246c8 golang.org/x/sync v0.0.0-20210220032951-036812b2e83c diff --git a/go.sum b/go.sum index 1e5fdd5434a40..983fcf52a7a5b 100644 --- a/go.sum +++ b/go.sum @@ -1231,8 +1231,9 @@ golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPh golang.org/x/crypto v0.0.0-20201002170205-7f63de1d35b0/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= golang.org/x/crypto v0.0.0-20210322153248-0c34fe9e7dc2/go.mod h1:T9bdIzuCu7OtxOm1hfPfRQxPLYneinmdGuTeoZ9dtd4= golang.org/x/crypto v0.0.0-20210421170649-83a5a9bb288b/go.mod h1:T9bdIzuCu7OtxOm1hfPfRQxPLYneinmdGuTeoZ9dtd4= -golang.org/x/crypto v0.0.0-20210817164053-32db794688a5 h1:HWj/xjIHfjYU5nVXpTM0s39J9CbLn7Cc5a7IC5rwsMQ= golang.org/x/crypto v0.0.0-20210817164053-32db794688a5/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc= +golang.org/x/crypto v0.0.0-20220525230936-793ad666bf5e h1:T8NU3HyQ8ClP4SEE+KbFlg6n0NhuTsN4MyznaarGsZM= +golang.org/x/crypto v0.0.0-20220525230936-793ad666bf5e/go.mod h1:IxCIyHEi3zRg3s0A5j5BB6A9Jmi73HwBIUl50j+osU4= golang.org/x/exp v0.0.0-20180321215751-8460e604b9de/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= golang.org/x/exp v0.0.0-20180807140117-3d87b88a115f/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= golang.org/x/exp v0.0.0-20190121172915-509febef88a4/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= diff --git a/manifests/base/application-controller/argocd-application-controller-statefulset.yaml b/manifests/base/application-controller/argocd-application-controller-statefulset.yaml index 7de68b3b42855..47498703ede77 100644 --- a/manifests/base/application-controller/argocd-application-controller-statefulset.yaml +++ b/manifests/base/application-controller/argocd-application-controller-statefulset.yaml @@ -136,12 +136,6 @@ spec: port: 8082 initialDelaySeconds: 5 periodSeconds: 10 - livenessProbe: - httpGet: - path: /healthz - port: 8082 - initialDelaySeconds: 5 - periodSeconds: 10 securityContext: runAsNonRoot: true readOnlyRootFilesystem: true diff --git a/manifests/core-install.yaml b/manifests/core-install.yaml index 9985bef52d352..b4dfb8d651542 100644 --- a/manifests/core-install.yaml +++ b/manifests/core-install.yaml @@ -9911,12 +9911,6 @@ spec: optional: true image: quay.io/argoproj/argocd:latest imagePullPolicy: Always - livenessProbe: - httpGet: - path: /healthz - port: 8082 - initialDelaySeconds: 5 - periodSeconds: 10 name: argocd-application-controller ports: - containerPort: 8082 diff --git a/manifests/ha/install.yaml b/manifests/ha/install.yaml index b0c01ce1b32b9..c4c1fe526b626 100644 --- a/manifests/ha/install.yaml +++ b/manifests/ha/install.yaml @@ -11281,12 +11281,6 @@ spec: optional: true image: quay.io/argoproj/argocd:latest imagePullPolicy: Always - livenessProbe: - httpGet: - path: /healthz - port: 8082 - initialDelaySeconds: 5 - periodSeconds: 10 name: argocd-application-controller ports: - containerPort: 8082 diff --git a/manifests/ha/namespace-install.yaml b/manifests/ha/namespace-install.yaml index 3f2ca84f556b2..fcf65d781158b 100644 --- a/manifests/ha/namespace-install.yaml +++ b/manifests/ha/namespace-install.yaml @@ -2136,12 +2136,6 @@ spec: optional: true image: quay.io/argoproj/argocd:latest imagePullPolicy: Always - livenessProbe: - httpGet: - path: /healthz - port: 8082 - initialDelaySeconds: 5 - periodSeconds: 10 name: argocd-application-controller ports: - containerPort: 8082 diff --git a/manifests/install.yaml b/manifests/install.yaml index 7161bdd60f439..f7d3095369b6d 100644 --- a/manifests/install.yaml +++ b/manifests/install.yaml @@ -10612,12 +10612,6 @@ spec: optional: true image: quay.io/argoproj/argocd:latest imagePullPolicy: Always - livenessProbe: - httpGet: - path: /healthz - port: 8082 - initialDelaySeconds: 5 - periodSeconds: 10 name: argocd-application-controller ports: - containerPort: 8082 diff --git a/manifests/namespace-install.yaml b/manifests/namespace-install.yaml index 2b74e5ed00801..f9cc5bacade8a 100644 --- a/manifests/namespace-install.yaml +++ b/manifests/namespace-install.yaml @@ -1467,12 +1467,6 @@ spec: optional: true image: quay.io/argoproj/argocd:latest imagePullPolicy: Always - livenessProbe: - httpGet: - path: /healthz - port: 8082 - initialDelaySeconds: 5 - periodSeconds: 10 name: argocd-application-controller ports: - containerPort: 8082 diff --git a/reposerver/repository/repository_test.go b/reposerver/repository/repository_test.go index c2f85588294a5..c38171c9e21a4 100644 --- a/reposerver/repository/repository_test.go +++ b/reposerver/repository/repository_test.go @@ -1130,6 +1130,7 @@ func TestListApps(t *testing.T) { "kustomization_yml": "Kustomize", "my-chart": "Helm", "my-chart-2": "Helm", + "values-files": "Helm", } assert.Equal(t, expectedApps, res.Apps) } @@ -1995,3 +1996,32 @@ func runGit(t *testing.T, workDir string, args ...string) string { require.NoError(t, err, stringOut) return stringOut } + +func Test_findHelmValueFilesInPath(t *testing.T) { + t.Run("does not exist", func(t *testing.T) { + files, err := findHelmValueFilesInPath("/obviously/does/not/exist") + assert.Error(t, err) + assert.Empty(t, files) + }) + t.Run("values files", func(t *testing.T) { + files, err := findHelmValueFilesInPath("./testdata/values-files") + assert.NoError(t, err) + assert.Len(t, files, 4) + }) +} + +func Test_populateHelmAppDetails(t *testing.T) { + res := apiclient.RepoAppDetailsResponse{} + q := apiclient.RepoServerAppDetailsQuery{ + Repo: &argoappv1.Repository{}, + Source: &argoappv1.ApplicationSource{ + Helm: &argoappv1.ApplicationSourceHelm{ValueFiles: []string{"exclude.yaml", "has-the-word-values.yaml"}}, + }, + } + appPath, err := filepath.Abs("./testdata/values-files/") + require.NoError(t, err) + err = populateHelmAppDetails(&res, appPath, appPath, &q) + require.NoError(t, err) + assert.Len(t, res.Helm.Parameters, 3) + assert.Len(t, res.Helm.ValueFiles, 4) +} diff --git a/reposerver/repository/testdata/values-files/Chart.yaml b/reposerver/repository/testdata/values-files/Chart.yaml new file mode 100644 index 0000000000000..0c9526bab8cc5 --- /dev/null +++ b/reposerver/repository/testdata/values-files/Chart.yaml @@ -0,0 +1,2 @@ +name: my-chart +version: 1.1.0 \ No newline at end of file diff --git a/reposerver/repository/testdata/values-files/caps-extn-values.YAML b/reposerver/repository/testdata/values-files/caps-extn-values.YAML new file mode 100644 index 0000000000000..e69de29bb2d1d diff --git a/reposerver/repository/testdata/values-files/exclude.yaml b/reposerver/repository/testdata/values-files/exclude.yaml new file mode 100644 index 0000000000000..c9bb22cd260fb --- /dev/null +++ b/reposerver/repository/testdata/values-files/exclude.yaml @@ -0,0 +1 @@ +exclude: yaml diff --git a/reposerver/repository/testdata/values-files/has-the-word-values.yaml b/reposerver/repository/testdata/values-files/has-the-word-values.yaml new file mode 100644 index 0000000000000..911b485e5ef9b --- /dev/null +++ b/reposerver/repository/testdata/values-files/has-the-word-values.yaml @@ -0,0 +1,4 @@ +has: + the: + word: + values: yaml diff --git a/reposerver/repository/testdata/values-files/values.yaml b/reposerver/repository/testdata/values-files/values.yaml new file mode 100644 index 0000000000000..55262d50ff71c --- /dev/null +++ b/reposerver/repository/testdata/values-files/values.yaml @@ -0,0 +1 @@ +values: yaml diff --git a/reposerver/repository/testdata/values-files/values.yml b/reposerver/repository/testdata/values-files/values.yml new file mode 100644 index 0000000000000..e69de29bb2d1d diff --git a/server/server_test.go b/server/server_test.go index 694fa4a4f2424..9ebd537236244 100644 --- a/server/server_test.go +++ b/server/server_test.go @@ -799,14 +799,15 @@ func TestAuthenticate_bad_request_metadata(t *testing.T) { }, } - ctx := context.Background() - for _, testData := range tests { testDataCopy := testData t.Run(testDataCopy.test, func(t *testing.T) { t.Parallel() + // Must be declared here to avoid race. + ctx := context.Background() //nolint:ineffassign,staticcheck + argocd, _ := getTestServer(t, testDataCopy.anonymousEnabled, true) ctx = metadata.NewIncomingContext(context.Background(), testDataCopy.metadata) diff --git a/test/container/Dockerfile b/test/container/Dockerfile index 29686ace8b7d8..b4ff7cdcbbc16 100644 --- a/test/container/Dockerfile +++ b/test/container/Dockerfile @@ -8,7 +8,7 @@ FROM docker.io/library/registry:2.8 as registry FROM docker.io/bitnami/kubectl:1.23.6 as kubectl -FROM ubuntu:21.10 +FROM ubuntu:22.04 ENV DEBIAN_FRONTEND=noninteractive RUN apt-get update && apt-get install --fix-missing -y \ @@ -64,6 +64,11 @@ COPY ./test/fixture/testrepos/ssh_host_*_key* /etc/ssh/ # Copy redis binaries to the image COPY --from=redis /usr/local/bin/* /usr/local/bin/ +# Copy redis dependencies/shared libraries +# Ubuntu 22.04+ has moved to OpenSSL3 and no longer provides these libraries +COPY --from=redis /usr/lib/x86_64-linux-gnu/libssl.so.1.1 /usr/lib/x86_64-linux-gnu/ +COPY --from=redis /usr/lib/x86_64-linux-gnu/libcrypto.so.1.1 /usr/lib/x86_64-linux-gnu/ + # Copy registry binaries to the image COPY --from=registry /bin/registry /usr/local/bin/ COPY --from=registry /etc/docker/registry/config.yml /etc/docker/registry/config.yml diff --git a/test/remote/Dockerfile b/test/remote/Dockerfile index 5b3d5d7ccc7ce..c96b26174110d 100644 --- a/test/remote/Dockerfile +++ b/test/remote/Dockerfile @@ -3,7 +3,7 @@ FROM golang:1.18 AS go RUN go install github.com/mattn/goreman@latest && \ go install github.com/kisielk/godepgraph@latest -FROM ubuntu:21.10 +FROM ubuntu:22.04 ENV DEBIAN_FRONTEND=noninteractive RUN apt-get update && apt-get install --no-install-recommends -y \ diff --git a/ui/src/app/settings/components/clusters-list/clusters-list.tsx b/ui/src/app/settings/components/clusters-list/clusters-list.tsx index e316974db0b38..c6dea9ab372aa 100644 --- a/ui/src/app/settings/components/clusters-list/clusters-list.tsx +++ b/ui/src/app/settings/components/clusters-list/clusters-list.tsx @@ -1,4 +1,5 @@ import {DropDownMenu, ErrorNotification, NotificationType} from 'argo-ui'; +import {Tooltip} from 'argo-ui'; import * as React from 'react'; import {RouteComponentProps} from 'react-router-dom'; import {clusterName, ConnectionStateIcon, DataLoader, EmptyState, Page} from '../../../shared/components'; @@ -36,9 +37,16 @@ export const ClustersList = (props: RouteComponentProps<{}>) => { onClick={() => ctx.navigation.goto(`./${encodeURIComponent(cluster.server)}`)}>
- {clusterName(cluster.name)} + + + {clusterName(cluster.name)} + +
+
+ + {cluster.server} +
-
{cluster.server}
{cluster.info.serverVersion}
{cluster.info.connectionState.status} diff --git a/util/git/client.go b/util/git/client.go index 1e9f3531febb5..98c40c20641ca 100644 --- a/util/git/client.go +++ b/util/git/client.go @@ -34,6 +34,8 @@ import ( "github.com/argoproj/argo-cd/v2/util/proxy" ) +var ErrInvalidRepoURL = fmt.Errorf("repo URL is invalid") + type RevisionMetadata struct { Author string Date time.Time @@ -136,9 +138,13 @@ func WithEventHandlers(handlers EventHandlers) ClientOpts { func NewClient(rawRepoURL string, creds Creds, insecure bool, enableLfs bool, proxy string, opts ...ClientOpts) (Client, error) { r := regexp.MustCompile("(/|:)") - root := filepath.Join(os.TempDir(), r.ReplaceAllString(NormalizeGitURL(rawRepoURL), "_")) + normalizedGitURL := NormalizeGitURL(rawRepoURL) + if normalizedGitURL == "" { + return nil, fmt.Errorf("repository %q cannot be initialized: %w", rawRepoURL, ErrInvalidRepoURL) + } + root := filepath.Join(os.TempDir(), r.ReplaceAllString(normalizedGitURL, "_")) if root == os.TempDir() { - return nil, fmt.Errorf("Repository '%s' cannot be initialized, because its root would be system temp at %s", rawRepoURL, root) + return nil, fmt.Errorf("repository %q cannot be initialized, because its root would be system temp at %s", rawRepoURL, root) } return NewClientExt(rawRepoURL, root, creds, insecure, enableLfs, proxy, opts...) } diff --git a/util/git/client_test.go b/util/git/client_test.go index 4addc82aa4413..6cd690764d32d 100644 --- a/util/git/client_test.go +++ b/util/git/client_test.go @@ -92,3 +92,9 @@ func Test_nativeGitClient_Fetch_Prune(t *testing.T) { err = client.Fetch("") assert.NoError(t, err) } + +func TestNewClient_invalidSSHURL(t *testing.T) { + client, err := NewClient("ssh://bitbucket.org:org/repo", NopCreds{}, false, false, "") + assert.Nil(t, client) + assert.ErrorIs(t, err, ErrInvalidRepoURL) +} diff --git a/util/git/ssh.go b/util/git/ssh.go index eb07a056b29f8..a1cb337a80e63 100644 --- a/util/git/ssh.go +++ b/util/git/ssh.go @@ -11,14 +11,14 @@ import ( // Unfortunately, crypto/ssh does not offer public constants or list for // this. var SupportedSSHKeyExchangeAlgorithms = []string{ - "diffie-hellman-group1-sha1", - "diffie-hellman-group14-sha1", + "curve25519-sha256", + "curve25519-sha256@libssh.org", "ecdh-sha2-nistp256", "ecdh-sha2-nistp384", "ecdh-sha2-nistp521", - "curve25519-sha256@libssh.org", - "diffie-hellman-group-exchange-sha1", "diffie-hellman-group-exchange-sha256", + "diffie-hellman-group14-sha256", + "diffie-hellman-group14-sha1", } // List of default key exchange algorithms to use. We use those that are diff --git a/util/gpg/gpg_test.go b/util/gpg/gpg_test.go index 119d25f31808d..18b2e0b3b8b05 100644 --- a/util/gpg/gpg_test.go +++ b/util/gpg/gpg_test.go @@ -28,10 +28,24 @@ var syncTestSources = map[string]string{ } // Helper function to create temporary GNUPGHOME -func initTempDir(tb testing.TB) string { - p := tb.TempDir() +func initTempDir(t *testing.T) string { + // Intentionally avoid using t.TempDir. That function creates really long paths, which can exceed the socket file + // path length on some OSes. The GPG tests rely on sockets. + p, err := ioutil.TempDir(os.TempDir(), "") + if err != nil { + panic(err) + } fmt.Printf("-> Using %s as GNUPGHOME\n", p) - os.Setenv(common.EnvGnuPGHome, p) + err = os.Setenv(common.EnvGnuPGHome, p) + if err != nil { + panic(err) + } + t.Cleanup(func() { + err := os.RemoveAll(p) + if err != nil { + panic(err) + } + }) return p } diff --git a/util/helm/helm.go b/util/helm/helm.go index 0c3a3a184e8de..6b6e109d1a8f8 100644 --- a/util/helm/helm.go +++ b/util/helm/helm.go @@ -6,7 +6,6 @@ import ( "net/url" "os" "os/exec" - "path" "strings" "github.com/ghodss/yaml" @@ -143,11 +142,10 @@ func (h *helm) GetParameters(valuesFiles []pathutil.ResolvedFilePath) (map[strin if err == nil && (parsedURL.Scheme == "http" || parsedURL.Scheme == "https") { fileValues, err = config.ReadRemoteFile(file) } else { - filePath := path.Join(h.cmd.WorkDir, file) - if _, err := os.Stat(filePath); os.IsNotExist(err) { + if _, err := os.Stat(file); os.IsNotExist(err) { continue } - fileValues, err = ioutil.ReadFile(filePath) + fileValues, err = ioutil.ReadFile(file) } if err != nil { return nil, fmt.Errorf("failed to read value file %s: %s", file, err) diff --git a/util/helm/helm_test.go b/util/helm/helm_test.go index 67199c19020e8..400b1132cc4ff 100644 --- a/util/helm/helm_test.go +++ b/util/helm/helm_test.go @@ -1,8 +1,11 @@ package helm import ( + "path/filepath" "testing" + "github.com/stretchr/testify/require" + "github.com/argoproj/argo-cd/v2/util/io/path" "github.com/argoproj/gitops-engine/pkg/utils/kube" @@ -51,11 +54,16 @@ func TestHelmTemplateParams(t *testing.T) { } func TestHelmTemplateValues(t *testing.T) { - h, err := NewHelmApp("./testdata/redis", []HelmRepository{}, false, "", "", false) + repoRoot := "./testdata/redis" + repoRootAbs, err := filepath.Abs(repoRoot) + require.NoError(t, err) + h, err := NewHelmApp(repoRootAbs, []HelmRepository{}, false, "", "", false) assert.NoError(t, err) + valuesPath, _, err := path.ResolveFilePath(repoRootAbs, repoRootAbs, "values-production.yaml", nil) + require.NoError(t, err) opts := TemplateOpts{ Name: "test", - Values: []path.ResolvedFilePath{"values-production.yaml"}, + Values: []path.ResolvedFilePath{valuesPath}, } objs, err := template(h, &opts) assert.Nil(t, err) @@ -72,33 +80,48 @@ func TestHelmTemplateValues(t *testing.T) { } func TestHelmGetParams(t *testing.T) { - h, err := NewHelmApp("./testdata/redis", nil, false, "", "", false) + repoRoot := "./testdata/redis" + repoRootAbs, err := filepath.Abs(repoRoot) + require.NoError(t, err) + h, err := NewHelmApp(repoRootAbs, nil, false, "", "", false) assert.NoError(t, err) params, err := h.GetParameters(nil) assert.Nil(t, err) slaveCountParam := params["cluster.slaveCount"] - assert.Equal(t, slaveCountParam, "1") + assert.Equal(t, "1", slaveCountParam) } func TestHelmGetParamsValueFiles(t *testing.T) { - h, err := NewHelmApp("./testdata/redis", nil, false, "", "", false) + repoRoot := "./testdata/redis" + repoRootAbs, err := filepath.Abs(repoRoot) + require.NoError(t, err) + h, err := NewHelmApp(repoRootAbs, nil, false, "", "", false) assert.NoError(t, err) - params, err := h.GetParameters([]path.ResolvedFilePath{"values-production.yaml"}) + valuesPath, _, err := path.ResolveFilePath(repoRootAbs, repoRootAbs, "values-production.yaml", nil) + require.NoError(t, err) + params, err := h.GetParameters([]path.ResolvedFilePath{valuesPath}) assert.Nil(t, err) slaveCountParam := params["cluster.slaveCount"] - assert.Equal(t, slaveCountParam, "3") + assert.Equal(t, "3", slaveCountParam) } func TestHelmGetParamsValueFilesThatExist(t *testing.T) { - h, err := NewHelmApp("./testdata/redis", nil, false, "", "", false) + repoRoot := "./testdata/redis" + repoRootAbs, err := filepath.Abs(repoRoot) + require.NoError(t, err) + h, err := NewHelmApp(repoRootAbs, nil, false, "", "", false) assert.NoError(t, err) - params, err := h.GetParameters([]path.ResolvedFilePath{"values-missing.yaml", "values-production.yaml"}) + valuesMissingPath, _, err := path.ResolveFilePath(repoRootAbs, repoRootAbs, "values-missing.yaml", nil) + require.NoError(t, err) + valuesProductionPath, _, err := path.ResolveFilePath(repoRootAbs, repoRootAbs, "values-production.yaml", nil) + require.NoError(t, err) + params, err := h.GetParameters([]path.ResolvedFilePath{valuesMissingPath, valuesProductionPath}) assert.Nil(t, err) slaveCountParam := params["cluster.slaveCount"] - assert.Equal(t, slaveCountParam, "3") + assert.Equal(t, "3", slaveCountParam) } func TestHelmTemplateReleaseNameOverwrite(t *testing.T) { diff --git a/util/io/path/resolved.go b/util/io/path/resolved.go index 0343a379f0494..abc3ff1de9750 100644 --- a/util/io/path/resolved.go +++ b/util/io/path/resolved.go @@ -11,6 +11,7 @@ import ( ) // ResolvedFilePath represents a resolved file path and intended to prevent unintentional use of not verified file path. +// It is always either a URL or an absolute path. type ResolvedFilePath string // resolveSymbolicLinkRecursive resolves the symlink path recursively to its