diff --git a/docs/howitworks.md b/docs/howitworks.md index 6731a0f5..e82bc494 100644 --- a/docs/howitworks.md +++ b/docs/howitworks.md @@ -153,6 +153,40 @@ data: POSTGRES_URL: cG9zdGdyZXM6Ly91c2VyOnBhc3NAaG9zdDo5NDQzL215LWRiP3NzbG1vZGU9cmVxdWlyZQ== ``` +##### URL encoded placeholders +When using plugin with a Helm chart, it is possible to use placeholders in values file. If a chart applies Helm's `urlquery` function to the value in order to safely include it in an URL, the placeholder will end up looking like this: `%3Cpath%3Asome%2Fpath%23secret-key%3E`. + +The plugin can handle this case by finding any url encoded placeholders (inline-path only), replacing them, and re-url encoding the result. + +For example, imagine that we have this value file: + +```yaml +redis: + external: + addr: "redis-master.harbor.svc.cluster.local" + password: +``` + +And that the Helm chart passes the password value through `urlquery`, combines it with other data into a connection string and then adds it to a ConfigMap or Secret looking like this: + +```yaml +data: + _REDIS_URL_CORE: >- + redis://:%3Cpath%3Akv%2Fdata%2Fconfig%2Fredis-pwd%23password%3E@redis-master.harbor.svc.cluster.local/0?idle_timeout_seconds=30 +``` + +The plugin will be able to find the placeholder `%3Cpath%3Akv%2Fdata%2Fconfig%2Fredis-pwd%23password%3E`, decode it, get the password value (for example, "redis@123"), re-encode the value as "redis%40123" and put it back in the connection string. + +Thus, the output will look like this: + +```yaml +data: + _REDIS_URL_CORE: >- + redis://:redis%40123@redis-master.harbor.svc.cluster.local/0?idle_timeout_seconds=30 +``` + +It will work even if the string with url-encoded placeholders was added to a Secret and base64-encoded. + ##### Automatically ignoring `` strings The plugin tries to be helpful and will ignore strings in the format `` if the `avp.kubernetes.io/path` annotation is missing, and only try to replace [inline-path placeholders](#inline-path-placeholders) diff --git a/pkg/kube/util.go b/pkg/kube/util.go index aeaccc20..91d7c1af 100644 --- a/pkg/kube/util.go +++ b/pkg/kube/util.go @@ -13,6 +13,7 @@ import ( "github.com/argoproj-labs/argocd-vault-plugin/pkg/types" "github.com/argoproj-labs/argocd-vault-plugin/pkg/utils" k8yaml "k8s.io/apimachinery/pkg/util/yaml" + "net/url" ) type missingKeyError struct { @@ -25,6 +26,7 @@ func (e *missingKeyError) Error() string { var genericPlaceholder, _ = regexp.Compile(`(?mU)<(.*)>`) var specificPathPlaceholder, _ = regexp.Compile(`(?mU)`) +var specificPathUrlEncodedPlaceholder, _ = regexp.Compile(`(?mU)%3Cpath%3A(.+)%23(.+)(?:%23(.+))?%3E`) var indivPlaceholderSyntax, _ = regexp.Compile(`(?mU)path:(?P[^#]+?)#(?P[^#]+?)(?:#(?P.+?))??`) // replaceInner recurses through the given map and replaces the placeholders by calling `replacerFunc` @@ -106,6 +108,29 @@ func genericReplacement(key, value string, resource Resource) (_ interface{}, er var nonStringReplacement interface{} var placeholderRegex = specificPathPlaceholder + // If some sepecific path placeholders were URL-encoded, we match them with a special regex, + // decode them, get secret values and re-encode them. + decodedValue := specificPathUrlEncodedPlaceholder.ReplaceAllFunc([]byte(value), func(match []byte) []byte { + decoded, decErr:= url.QueryUnescape(string(match)) + if decErr != nil || !placeholderRegex.Match([]byte(decoded)) { + err = append(err, decErr) + return match + } + + repl, replErr := genericReplacement(key, decoded, resource) + if replErr != nil { + err = append(err, replErr...) + return match + } + + return []byte(url.QueryEscape(stringify(repl))) + }) + + if string(decodedValue) != value { + utils.VerboseToStdErr("key %s had value with URL encoded placeholder", key) + value = string(decodedValue) + } + // If the Vault path annotation is present, there may be placeholders with/without an explicit path // so we look for those. Only if the annotation is absent do we narrow the search to placeholders with // explicit paths, to prevent catching that aren't placeholders @@ -216,7 +241,7 @@ func configReplacement(key, value string, resource Resource) (interface{}, []err func secretReplacement(key, value string, resource Resource) (interface{}, []error) { decoded, err := base64.StdEncoding.DecodeString(value) - if err == nil && genericPlaceholder.Match(decoded) { + if err == nil && (genericPlaceholder.Match(decoded) || specificPathUrlEncodedPlaceholder.Match(decoded)) { res, err := genericReplacement(key, string(decoded), resource) utils.VerboseToStdErr("key %s comes from Secret manifest, base64 encoding value %s to fit", key, value) diff --git a/pkg/kube/util_test.go b/pkg/kube/util_test.go index b09fef90..b1ba5041 100644 --- a/pkg/kube/util_test.go +++ b/pkg/kube/util_test.go @@ -195,6 +195,131 @@ func TestGenericReplacement_specificPathWithValidation(t *testing.T) { }) } +func TestGenericReplacement_specificPathUrlEncoded(t *testing.T) { + // Test that the generic replacement function can find/replace placeholders that were url-encoded + mv := helpers.MockVault{} + mv.LoadData(map[string]interface{}{ + "namespace": "default ns", + }) + + dummyResource := Resource{ + TemplateData: map[string]interface{}{ + "namespace": "%3Cpath%3Ablah%2Fblah%23namespace%3E", + "name": "", + }, + Data: map[string]interface{}{ + "namespace": "something-else", + "name": "foo", + }, + Backend: &mv, + Annotations: map[string]string{ + (types.AVPPathAnnotation): "", + }, + } + + replaceInner(&dummyResource, &dummyResource.TemplateData, genericReplacement) + + if !mv.GetIndividualSecretCalled { + t.Fatalf("expected GetSecrets to be called since placeholder contains explicit path so Vault lookup is neeed") + } + + expected := Resource{ + TemplateData: map[string]interface{}{ + "namespace": "default+ns", + "name": "foo", + }, + Data: map[string]interface{}{ + "namespace": "something-else", + "name": "foo", + }, + replacementErrors: []error{}, + } + + assertSuccessfulReplacement(&dummyResource, &expected, t) +} + +func TestGenericReplacement_specificPathUrlEncodedWithValidation(t *testing.T) { + // Test that the generic replacement function can find/replace placeholders that were url-encoded + mv := helpers.MockVault{} + mv.LoadData(map[string]interface{}{ + "namespace": "default ns", + }) + + t.Run("valid path", func(t *testing.T) { + dummyResource := Resource{ + TemplateData: map[string]interface{}{ + "namespace": "%3Cpath%3Ablah%2Fblah%23namespace%3E", + "name": "", + }, + Data: map[string]interface{}{ + "namespace": "something-else", + "name": "foo", + }, + Backend: &mv, + Annotations: map[string]string{ + (types.AVPPathAnnotation): "", + }, + PathValidation: regexp.MustCompile(`^([A-Za-z/]*)$`), + } + + replaceInner(&dummyResource, &dummyResource.TemplateData, genericReplacement) + + if !mv.GetIndividualSecretCalled { + t.Fatalf("expected GetSecrets to be called since placeholder contains explicit path so Vault lookup is neeed") + } + + expected := Resource{ + TemplateData: map[string]interface{}{ + "namespace": "default+ns", + "name": "foo", + }, + Data: map[string]interface{}{ + "namespace": "something-else", + "name": "foo", + }, + replacementErrors: []error{}, + } + + assertSuccessfulReplacement(&dummyResource, &expected, t) + }) + + t.Run("invalid path", func(t *testing.T) { + dummyResource := Resource{ + TemplateData: map[string]interface{}{ + "namespace": "%3Cpath%3A..%2Fblah%2Fblah%23namespace%3E", + }, + Data: map[string]interface{}{ + "namespace": "something-else", + }, + Backend: &mv, + Annotations: map[string]string{ + (types.AVPPathAnnotation): "", + }, + PathValidation: regexp.MustCompile(`^([A-Za-z/]*)$`), + } + + replaceInner(&dummyResource, &dummyResource.TemplateData, genericReplacement) + + if !mv.GetIndividualSecretCalled { + t.Fatalf("expected GetSecrets to be called since placeholder contains explicit path so Vault lookup is neeed") + } + + expected := Resource{ + TemplateData: map[string]interface{}{ + "namespace": "%3Cpath%3A..%2Fblah%2Fblah%23namespace%3E", + }, + Data: map[string]interface{}{ + "namespace": "something-else", + }, + replacementErrors: []error{ + fmt.Errorf("the path ../blah/blah is disallowed by AVP_PATH_VALIDATION restriction"), + }, + } + + assertFailedReplacement(&dummyResource, &expected, t) + }) +} + func TestGenericReplacement_specificPathVersioned(t *testing.T) { // Test that the specific-path placeholder syntax with versioning is used to find/replace placeholders mv := helpers.MockVault{} @@ -316,6 +441,87 @@ func TestGenericReplacement_multiString(t *testing.T) { assertSuccessfulReplacement(&dummyResource, &expected, t) } +func TestGenericReplacement_multiStringSpecificPathUrlEncoded(t *testing.T) { + // Test that multiple url-encoded placeholders in one value string can all be found/replaced. + mv := helpers.MockVault{} + mv.LoadData(map[string]interface{}{ + "name": "my app", + "tag": "v1", + }) + + dummyResource := Resource{ + TemplateData: map[string]interface{}{ + "namespace": "", + "image": "foo.io/%3Cpath%3Ablah%2Fblah%23name%3E:%3Cpath%3Ablah%2Fblah%23tag%3E", + }, + Data: map[string]interface{}{ + "namespace": "default", + "name": "app", + "tag": "latest", + }, + Backend: &mv, + Annotations: map[string]string{ + (types.AVPPathAnnotation): "", + }, + } + + replaceInner(&dummyResource, &dummyResource.TemplateData, genericReplacement) + + expected := Resource{ + TemplateData: map[string]interface{}{ + "namespace": "default", + "image": "foo.io/my+app:v1", + }, + Data: map[string]interface{}{ + "namespace": "default", + "name": "app", + "tag": "latest", + }, + replacementErrors: []error{}, + } + + assertSuccessfulReplacement(&dummyResource, &expected, t) +} + +func TestSecretReplacement_SpecificPathUrlEncoded_Base64Encoded(t *testing.T) { + // Test that the secret replacement function can find/replace placeholders that were url-encoded + // and then base64-encoded. + mv := helpers.MockVault{} + mv.LoadData(map[string]interface{}{ + "password": "redis@123", + "username": "redis", + }) + + dummyResource := Resource{ + TemplateData: map[string]interface{}{ + "url": `cmVkaXM6Ly8lM0NwYXRoJTNBYmxhaCUyRmJsYWglMjN1c2VybmFtZSUzRTolM0NwYXRoJTNBYmxhaCUyRmJsYWglMjNwYXNzd29yZCUzRUByZWRpcy1tYXN0ZXIuaGFyYm9yLnN2Yy5jbHVzdGVyLmxvY2FsLzA/aWRsZV90aW1lb3V0X3NlY29uZHM9MzAK`, + }, + Data: map[string]interface{}{ + "password": "test", + "username": "test", + }, + Backend: &mv, + Annotations: map[string]string{ + (types.AVPPathAnnotation): "", + }, + } + + replaceInner(&dummyResource, &dummyResource.TemplateData, secretReplacement) + + expected := Resource{ + TemplateData: map[string]interface{}{ + "url": "cmVkaXM6Ly9yZWRpczpyZWRpcyU0MDEyM0ByZWRpcy1tYXN0ZXIuaGFyYm9yLnN2Yy5jbHVzdGVyLmxvY2FsLzA/aWRsZV90aW1lb3V0X3NlY29uZHM9MzAK", + }, + Data: map[string]interface{}{ + "password": "test", + "username": "test", + }, + replacementErrors: []error{}, + } + + assertSuccessfulReplacement(&dummyResource, &expected, t) +} + func TestGenericReplacement_Base64(t *testing.T) { dummyResource := Resource{ TemplateData: map[string]interface{}{