diff --git a/pkg/util/selector/prr_selector.go b/pkg/util/selector/prr_selector.go index 871b4916d..2eeb7eb1a 100644 --- a/pkg/util/selector/prr_selector.go +++ b/pkg/util/selector/prr_selector.go @@ -41,12 +41,53 @@ func ParsePRRGet(rawName string) (packageRevisionName string, selector PRRGet, e if err != nil { return "", PRRGet{}, err } - files := queryValues[fileQueryKey] + files, err := decodeFilePaths(queryValues[fileQueryKey]) + if err != nil { + return "", PRRGet{}, err + } return packageRevisionName, PRRGet{ FilePaths: files, }, nil } +// decodeFilePaths decodes a list of "file" query values, where ":" stands in +// for "/" so nested paths can be passed as a Kubernetes resource name (which +// may not contain "/"). A literal ":" or "\" is written as "\:" or "\\". +func decodeFilePaths(rawFilePaths []string) ([]string, error) { + if rawFilePaths == nil { + return nil, nil + } + filePaths := make([]string, len(rawFilePaths)) + for i, rawFilePath := range rawFilePaths { + filePath, err := decodeFilePath(rawFilePath) + if err != nil { + return nil, err + } + filePaths[i] = filePath + } + return filePaths, nil +} + +func decodeFilePath(rawFilePath string) (string, error) { + var decoded strings.Builder + for i := 0; i < len(rawFilePath); i++ { + c := rawFilePath[i] + switch c { + case '\\': + i++ + if i >= len(rawFilePath) || (rawFilePath[i] != ':' && rawFilePath[i] != '\\') { + return "", pkgerrors.Errorf("invalid escape sequence in file path %q", rawFilePath) + } + decoded.WriteByte(rawFilePath[i]) + case ':': + decoded.WriteByte('/') + default: + decoded.WriteByte(c) + } + } + return decoded.String(), nil +} + func ParsePRRUpdate(rawName string) (packageRevisionName string, selector PRRUpdate, err error) { packageRevisionName, queryValues, err := parseRawName(rawName) if err != nil { diff --git a/pkg/util/selector/prr_selector_test.go b/pkg/util/selector/prr_selector_test.go index 766b0c1bc..003055497 100644 --- a/pkg/util/selector/prr_selector_test.go +++ b/pkg/util/selector/prr_selector_test.go @@ -14,6 +14,58 @@ const ( testReadmeFile = "README.md" ) +func TestDecodeFilePath(t *testing.T) { + testCases := map[string]struct { + rawFilePath string + expectedFilePath string + expectedErr string + }{ + "no colon": { + rawFilePath: testKptFile, + expectedFilePath: testKptFile, + }, + "nested path": { + rawFilePath: "deployments:nginx.yaml", + expectedFilePath: "deployments/nginx.yaml", + }, + "deeply nested path": { + rawFilePath: "a:b:c.yaml", + expectedFilePath: "a/b/c.yaml", + }, + "escaped colon": { + rawFilePath: `file\:name.yaml`, + expectedFilePath: "file:name.yaml", + }, + "escaped backslash": { + rawFilePath: `dir:file\\name.yaml`, + expectedFilePath: `dir/file\name.yaml`, + }, + "invalid escape sequence": { + rawFilePath: `file\nname.yaml`, + expectedErr: `invalid escape sequence in file path "file\\nname.yaml"`, + }, + "trailing backslash": { + rawFilePath: `file.yaml\`, + expectedErr: `invalid escape sequence in file path "file.yaml\\"`, + }, + } + + for tn, tc := range testCases { + t.Run(tn, func(t *testing.T) { + // when + filePath, err := decodeFilePath(tc.rawFilePath) + + // then + if tc.expectedErr == "" { + require.NoError(t, err, "expected no error") + assert.Equal(t, tc.expectedFilePath, filePath, "expected decoded file path does not match") + } else { + require.EqualError(t, err, tc.expectedErr, "expected error does not match") + } + }) + } +} + func TestParseGetPackageRevisionResourcesUrl(t *testing.T) { testCases := map[string]struct { nameWithQuery string diff --git a/test/e2e/api/advanced_test.go b/test/e2e/api/advanced_test.go index 7bfa8c31f..cb2dd50b2 100644 --- a/test/e2e/api/advanced_test.go +++ b/test/e2e/api/advanced_test.go @@ -676,6 +676,41 @@ func (t *PorchSuite) TestPackageMetadataFromKptfile() { }) } +// TestGetPackageRevisionResourcesNestedFile verifies that a single file in a +// subdirectory can be fetched via the "file" query selector by encoding "/" +// as ":" (e.g. "manifests:configmap.yaml" selects "manifests/configmap.yaml"). +func (t *PorchSuite) TestGetPackageRevisionResourcesNestedFile() { + const ( + repositoryName = "test-nested-file-repo" + packageName = "test-nested-file" + nestedFilePath = "manifests/configmap.yaml" + nestedFileSelector = "manifests:configmap.yaml" + ) + + t.RegisterGitRepositoryF(t.GetPorchTestRepoURL(), repositoryName, "", t.GiteaUser, suiteutils.Password(t.GiteaPassword)) + pr := t.CreatePackageDraftF(repositoryName, packageName, defaultWorkspace) + + resources := t.WaitUntilPackageRevisionResourcesExists(types.NamespacedName{Namespace: t.Namespace, Name: pr.Name}) + resources.Spec.Resources[nestedFilePath] = `apiVersion: v1 +kind: ConfigMap +metadata: + name: nested-cm +data: + key: nested-value +` + t.UpdateF(resources) + + var packageResources porchapi.PackageRevisionResources + key := client.ObjectKeyFromObject(resources) + key.Name = fmt.Sprintf("%s?file=%s", key.Name, nestedFileSelector) + t.GetF(key, &packageResources) + + t.Require().Len(packageResources.Spec.Resources, 1) + content, ok := packageResources.Spec.Resources[nestedFilePath] + t.Require().True(ok, "expected nested file %q to be returned under its full path", nestedFilePath) + t.Require().Contains(content, "nested-cm") +} + func (t *PorchSuite) TestPackageMetadataFieldSelectors() { const ( repositoryName = "test-package-field-selector-repo"