diff --git a/metadata/registry_managers.go b/metadata/registry_managers.go index aaeb82a0e0ad4f34a1ce1b16fe7ff102f6f95564..97c48168c3ebf8b8d92ce702a6412c363d0c81fc 100644 --- a/metadata/registry_managers.go +++ b/metadata/registry_managers.go @@ -26,6 +26,10 @@ const ( resolvedSHAField = "resolvedSHA" ) +var ( + errInvalidURI = fmt.Errorf("Invalid GitHub URI: try navigating in GitHub to the URI of the folder containing the 'registry.yaml', and using that URI instead. Generally, this URI should be of the form 'github.com/{organization}/{repository}/tree/{branch}/[path-to-directory]'") +) + // // GitHub registry manager. // @@ -275,6 +279,10 @@ func (gh *gitHubRegistryManager) registrySpecRawURL() string { return strings.Join([]string{rawGitHubRoot, gh.org, gh.repo, gh.GitVersion.RefSpec, gh.registrySpecRepoPath}, "/") } +// +// Helper functions. +// + func parseGitHubURI(uri string) (org, repo, refSpec, regRepoPath, regSpecRepoPath string, err error) { // Normalize URI. uri = strings.TrimSpace(uri) @@ -310,7 +318,7 @@ func parseGitHubURI(uri string) (org, repo, refSpec, regRepoPath, regSpecRepoPat // * URI points at a directory inside the respoitory, e.g., // 'http://github.com/ksonnet/parts/tree/master/incubator' // * URI points at an 'app.yaml', e.g., - // 'http://github.com/ksonnet/parts/blob/master/app.yaml' + // 'http://github.com/ksonnet/parts/blob/master/registry.yaml' // * URI points at a repository root, e.g., // 'http://github.com/ksonnet/parts' // @@ -324,13 +332,13 @@ func parseGitHubURI(uri string) (org, repo, refSpec, regRepoPath, regSpecRepoPat // See note above about first component being blank. if components[3] == "tree" { - regRepoPath = strings.Join(components[5:], "/") - - // If we have a trailing '/' character, last component will be - // blank. + // If we have a trailing '/' character, last component will be blank. Make + // sure that `regRepoPath` does not contain a trailing `/`. if components[len-1] == "" { + regRepoPath = strings.Join(components[5:len-1], "/") components[len-1] = registryYAMLFile } else { + regRepoPath = strings.Join(components[5:], "/") components = append(components, registryYAMLFile) } regSpecRepoPath = strings.Join(components[5:], "/") @@ -341,7 +349,7 @@ func parseGitHubURI(uri string) (org, repo, refSpec, regRepoPath, regSpecRepoPat regSpecRepoPath = strings.Join(components[5:], "/") return } else { - return "", "", "", "", "", fmt.Errorf("Invalid GitHub URI: try navigating in GitHub to the URI of the folder containing the 'app.yaml', and using that URI instead. Generally, this URI should be of the form 'github.com/{organization}/{repository}/tree/{branch}/[path-to-directory]'") + return "", "", "", "", "", errInvalidURI } } else { refSpec = defaultGitHubBranch diff --git a/metadata/registry_test.go b/metadata/registry_test.go index 8248681e682625127aaf0c1fb11375f9651d7e9e..f69a31a47e48cb8196571c030d3e8752c0c2aa82 100644 --- a/metadata/registry_test.go +++ b/metadata/registry_test.go @@ -2,12 +2,164 @@ package metadata import ( "path" + "testing" "github.com/ksonnet/ksonnet/metadata/app" "github.com/ksonnet/ksonnet/metadata/parts" "github.com/ksonnet/ksonnet/metadata/registry" ) +func TestParseGiHubRegistryURITest(t *testing.T) { + tests := []struct { + // Specification to parse. + uri string + + // Optional error to check. + targetErr error + + // Optional results to verify. + targetOrg string + targetRepo string + targetRefSpec string + targetRegistryRepoPath string + targetRegistrySpecRepoPath string + }{ + // + // `parseGitHubURI` should correctly parse org, repo, and refspec. Does not + // test path parsing. + // + { + uri: "github.com/exampleOrg1/exampleRepo1", + + targetOrg: "exampleOrg1", + targetRepo: "exampleRepo1", + targetRefSpec: "master", + targetRegistryRepoPath: "", + targetRegistrySpecRepoPath: "registry.yaml", + }, + { + uri: "github.com/exampleOrg2/exampleRepo2/tree/master", + + targetOrg: "exampleOrg2", + targetRepo: "exampleRepo2", + targetRefSpec: "master", + targetRegistryRepoPath: "", + targetRegistrySpecRepoPath: "registry.yaml", + }, + { + uri: "github.com/exampleOrg3/exampleRepo3/tree/exampleBranch1", + + targetOrg: "exampleOrg3", + targetRepo: "exampleRepo3", + targetRefSpec: "exampleBranch1", + targetRegistryRepoPath: "", + targetRegistrySpecRepoPath: "registry.yaml", + }, + { + // Fails because `blob` refers to a file, but this refers to a directory. + uri: "github.com/exampleOrg4/exampleRepo4/blob/master", + targetErr: errInvalidURI, + }, + { + uri: "github.com/exampleOrg4/exampleRepo4/tree/exampleBranch2", + + targetOrg: "exampleOrg4", + targetRepo: "exampleRepo4", + targetRefSpec: "exampleBranch2", + targetRegistryRepoPath: "", + targetRegistrySpecRepoPath: "registry.yaml", + }, + + // + // Parsing URIs with paths. + // + { + // Fails because referring to a directory requires a URI with + // `tree/{branchName}` prepending the path. + uri: "github.com/exampleOrg6/exampleRepo6/path/to/some/registry", + targetErr: errInvalidURI, + }, + { + uri: "github.com/exampleOrg5/exampleRepo5/tree/master/path/to/some/registry", + + targetOrg: "exampleOrg5", + targetRepo: "exampleRepo5", + targetRefSpec: "master", + targetRegistryRepoPath: "path/to/some/registry", + targetRegistrySpecRepoPath: "path/to/some/registry/registry.yaml", + }, + { + uri: "github.com/exampleOrg6/exampleRepo6/tree/exampleBranch3/path/to/some/registry", + + targetOrg: "exampleOrg6", + targetRepo: "exampleRepo6", + targetRefSpec: "exampleBranch3", + targetRegistryRepoPath: "path/to/some/registry", + targetRegistrySpecRepoPath: "path/to/some/registry/registry.yaml", + }, + { + // Fails because `blob` refers to a file, but this refers to a directory. + uri: "github.com/exampleOrg7/exampleRepo7/blob/master", + targetErr: errInvalidURI, + }, + { + // Fails because `blob` refers to a file, but this refers to a directory. + uri: "github.com/exampleOrg5/exampleRepo5/blob/exampleBranch2", + targetErr: errInvalidURI, + }, + } + + for _, test := range tests { + // Make sure we correctly parse each URN as a bare-domain URI, as well as + // with 'http://' and 'https://' as prefixes. + for _, prefix := range []string{"http://", "https://", "http://www.", "https://www.", "www.", ""} { + // Make sure we correctly parse each URI even if it has the optional + // trailing `/` character. + for _, suffix := range []string{"/", ""} { + uri := prefix + test.uri + suffix + + t.Run(uri, func(t *testing.T) { + org, repo, refspec, registryRepoPath, registrySpecRepoPath, err := parseGitHubURI(uri) + if test.targetErr != nil { + if err != test.targetErr { + t.Fatalf("Expected URI '%s' parse to fail with err '%v', got: '%v'", uri, test.targetErr, err) + } + return + } + + if err != nil { + t.Fatalf("Expected parse to succeed, but failed with error '%v'", err) + } + + if org != test.targetOrg { + t.Errorf("Expected org '%s', got '%s'", test.targetOrg, org) + } + + if repo != test.targetRepo { + t.Errorf("Expected repo '%s', got '%s'", test.targetRepo, repo) + } + + if refspec != test.targetRefSpec { + t.Errorf("Expected refspec '%s', got '%s'", test.targetRefSpec, refspec) + } + + if registryRepoPath != test.targetRegistryRepoPath { + t.Errorf("Expected registryRepoPath '%s', got '%s'", test.targetRegistryRepoPath, registryRepoPath) + } + + if registrySpecRepoPath != test.targetRegistrySpecRepoPath { + t.Errorf("Expected targetRegistrySpecRepoPath '%s', got '%s'", test.targetRegistrySpecRepoPath, registrySpecRepoPath) + } + }) + } + } + } +} + +// +// Mock registry manager for end-to-end tests. +// + type mockRegistryManager struct { *app.RegistryRefSpec registryDir string