Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

✨ clusterctl: Add support for valid GitHub URLs #11595

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

✨ clusterctl: Add support for valid GitHub URLs #11595

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d4aa83d
GitHub repo: Add support for valid GitHub URLs
HomayoonAlimohammadi Dec 18, 2024
4dd956a
Fix lint
HomayoonAlimohammadi Dec 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 41 additions & 6 deletions cmd/clusterctl/client/repository/repository_github.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,12 +45,17 @@ const (
httpsScheme = "https"
githubDomain = "github.com"
githubReleaseRepository = "releases"
githubReleaseDownload = "download"
githubLatestReleaseLabel = "latest"
githubListReleasesPerPageLimit = 100
)

var (
errNotFound = errors.New("404 Not Found")
errNotFound = errors.New("404 Not Found")
errInvalidURLFormat = errors.Errorf(
"invalid url: a GitHub repository url should either be in form https://github.com/{owner}/{Repository}/%[1]s/%[2]s/%[3]s/{componentsClient.yaml} or https://github.com/{owner}/{Repository}/%[1]s/%[3]s/{versionTag}/{componentsClient.yaml} or https://github.com/{owner}/{Repository}/%[1]s/{latest|version-tag}/{componentsClient.yaml} (old format)",
githubReleaseRepository, githubLatestReleaseLabel, githubReleaseDownload,
)

// Caches used to limit the number of GitHub API calls.

Expand Down Expand Up @@ -197,6 +202,27 @@ func (g *gitHubRepository) GetFile(ctx context.Context, version, path string) ([
return files, nil
}

func isURLSplitValid(s []string) bool {
return isURLSplitValidWithoutDownload(s) || isURLSplitValidWithDownload(s)
}

// isURLSplitValidWithDownload checks if the url split is valid.
// The url split should be in one of the following two forms:
// 1. https://github.com/{owner}/{Repository}/releases/latest/download/{[path/to/]componentsClient.yaml}
// 2. https://github.com/{owner}/{Repository}/releases/download/{versionTag}/{[path/to/]componentsClient.yaml}
//
// The only difference being the order of "releases", "download" and "tag|latest".
// It's worth noting that the first one will get redirected to the second one with an HTTP 302.
func isURLSplitValidWithDownload(s []string) bool {
return len(s) >= 6 &&
((s[2] == githubReleaseRepository && s[3] == githubLatestReleaseLabel && s[4] == githubReleaseDownload) ||
(s[2] == githubReleaseRepository && s[3] == githubReleaseDownload))
}

func isURLSplitValidWithoutDownload(s []string) bool {
return len(s) >= 5 && s[2] == githubReleaseRepository
}

// NewGitHubRepository returns a gitHubRepository implementation.
func NewGitHubRepository(ctx context.Context, providerConfig config.Provider, configVariablesClient config.VariablesClient, opts ...githubRepositoryOption) (Repository, error) {
if configVariablesClient == nil {
Expand All @@ -216,19 +242,28 @@ func NewGitHubRepository(ctx context.Context, providerConfig config.Provider, co
// Check if the path is in the expected format,
// url's path has an extra leading slash at the end which we need to clean up before splitting.
urlSplit := strings.Split(strings.TrimPrefix(rURL.Path, "/"), "/")
if len(urlSplit) < 5 || urlSplit[2] != githubReleaseRepository {
return nil, errors.Errorf(
"invalid url: a GitHub repository url should be in the form https://github.com/{owner}/{Repository}/%s/{latest|version-tag}/{componentsClient.yaml}",
githubReleaseRepository,
)
if !isURLSplitValid(urlSplit) {
return nil, errInvalidURLFormat
}

// Extract all the info from url split.
owner := urlSplit[0]
repository := urlSplit[1]
// urlSplit[2] == "releases"
defaultVersion := urlSplit[3]
path := strings.Join(urlSplit[4:], "/")

// In case we have the url with "download", we need to adjust the defaultVersion and path.
if urlSplit[3] == githubReleaseDownload || urlSplit[4] == githubReleaseDownload {
// urlSplit[3] == "download" || "latest"
// urlSplit[4] == $versionTag || "download"
defaultVersion = githubLatestReleaseLabel
if urlSplit[3] == githubReleaseDownload {
defaultVersion = urlSplit[4]
}
path = strings.Join(urlSplit[5:], "/")
}

// Use path's directory as a rootPath.
rootPath := filepath.Dir(path)
// Use the file name (if any) as componentsPath.
Expand Down
148 changes: 137 additions & 11 deletions cmd/clusterctl/client/repository/repository_github_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"context"
"fmt"
"net/http"
"path"
"strings"
"testing"
"time"
Expand Down Expand Up @@ -136,8 +137,39 @@ func Test_gitHubRepository_GetVersions(t *testing.T) {
}

func Test_githubRepository_newGitHubRepository(t *testing.T) {
retryableOperationInterval = 200 * time.Millisecond
retryableOperationTimeout = 1 * time.Second
retryableOperationInterval = 100 * time.Millisecond
retryableOperationTimeout = 200 * time.Millisecond

client, mux, teardown := test.NewFakeGitHub()
defer teardown()

var (
owner = "o"
repo = "r1"
version = "v0.4.1"
componentDir = "path/to"
componentName = "component.yaml"
assetID = 123
)

// Setup a handler for returning fake releases in a paginated manner
mux.HandleFunc(fmt.Sprintf("/repos/%s/%s/releases", owner, repo), func(w http.ResponseWriter, r *http.Request) {
goproxytest.HTTPTestMethod(t, r, "GET")
fmt.Fprint(w, `[`)
fmt.Fprintf(w, `{"id":1, "tag_name": "%s"}`, version)
fmt.Fprint(w, `]`)
})

mux.HandleFunc(fmt.Sprintf("/repos/%s/%s/releases/tags/%s", owner, repo, version), func(w http.ResponseWriter, r *http.Request) {
goproxytest.HTTPTestMethod(t, r, "GET")
fmt.Fprintf(w, `{"tag_name": "%s", "assets": [{"id": %d, "name": "%s"}]}`, version, assetID, path.Join(componentDir, "metadata.yaml"))
})

mux.HandleFunc(fmt.Sprintf("/repos/%s/%s/releases/assets/%d", owner, repo, assetID), func(w http.ResponseWriter, r *http.Request) {
goproxytest.HTTPTestMethod(t, r, "GET")
fmt.Fprint(w, `random file content`)
})

type field struct {
providerConfig config.Provider
variableClient config.VariablesClient
Expand All @@ -151,19 +183,19 @@ func Test_githubRepository_newGitHubRepository(t *testing.T) {
{
name: "can create a new GitHub repo",
field: field{
providerConfig: config.NewProvider("test", "https://github.com/o/r1/releases/v0.4.1/path", clusterctlv1.CoreProviderType),
providerConfig: config.NewProvider("test", fmt.Sprintf("https://github.com/%s/%s/releases/%s/%s", owner, repo, version, path.Join(componentDir, componentName)), clusterctlv1.CoreProviderType),
variableClient: test.NewFakeVariableClient(),
},
want: &gitHubRepository{
providerConfig: config.NewProvider("test", "https://github.com/o/r1/releases/v0.4.1/path", clusterctlv1.CoreProviderType),
providerConfig: config.NewProvider("test", fmt.Sprintf("https://github.com/%s/%s/releases/%s/%s", owner, repo, version, path.Join(componentDir, componentName)), clusterctlv1.CoreProviderType),
configVariablesClient: test.NewFakeVariableClient(),
authenticatingHTTPClient: nil,
owner: "o",
repository: "r1",
defaultVersion: "v0.4.1",
rootPath: ".",
componentsPath: "path",
injectClient: nil,
owner: owner,
repository: repo,
defaultVersion: version,
rootPath: componentDir,
componentsPath: componentName,
injectClient: client,
},
wantErr: false,
},
Expand Down Expand Up @@ -212,14 +244,108 @@ func Test_githubRepository_newGitHubRepository(t *testing.T) {
want: nil,
wantErr: true,
},
{
name: "valid url without `download`, with `latest`",
field: field{
providerConfig: config.NewProvider("test", fmt.Sprintf("https://github.com/%s/%s/releases/latest/%s", owner, repo, path.Join(componentDir, componentName)), clusterctlv1.CoreProviderType),
variableClient: test.NewFakeVariableClient(),
},
want: &gitHubRepository{
providerConfig: config.NewProvider("test", fmt.Sprintf("https://github.com/%s/%s/releases/latest/%s", owner, repo, path.Join(componentDir, componentName)), clusterctlv1.CoreProviderType),
configVariablesClient: test.NewFakeVariableClient(),
authenticatingHTTPClient: nil,
owner: owner,
repository: repo,
defaultVersion: version,
rootPath: componentDir,
componentsPath: componentName,
injectClient: client,
},
wantErr: false,
},
{
name: "valid url with `download`, with `latest`",
field: field{
providerConfig: config.NewProvider("test", fmt.Sprintf("https://github.com/%s/%s/releases/latest/download/%s", owner, repo, path.Join(componentDir, componentName)), clusterctlv1.CoreProviderType),
variableClient: test.NewFakeVariableClient(),
},
want: &gitHubRepository{
providerConfig: config.NewProvider("test", fmt.Sprintf("https://github.com/%s/%s/releases/latest/download/%s", owner, repo, path.Join(componentDir, componentName)), clusterctlv1.CoreProviderType),
configVariablesClient: test.NewFakeVariableClient(),
authenticatingHTTPClient: nil,
owner: owner,
repository: repo,
defaultVersion: version,
rootPath: componentDir,
componentsPath: componentName,
injectClient: client,
},
wantErr: false,
},
{
name: "valid url with `download`, with version",
field: field{
providerConfig: config.NewProvider("test", fmt.Sprintf("https://github.com/%s/%s/releases/download/%s/%s", owner, repo, version, path.Join(componentDir, componentName)), clusterctlv1.CoreProviderType),
variableClient: test.NewFakeVariableClient(),
},
want: &gitHubRepository{
providerConfig: config.NewProvider("test", fmt.Sprintf("https://github.com/%s/%s/releases/download/%s/%s", owner, repo, version, path.Join(componentDir, componentName)), clusterctlv1.CoreProviderType),
configVariablesClient: test.NewFakeVariableClient(),
authenticatingHTTPClient: nil,
owner: owner,
repository: repo,
defaultVersion: version,
rootPath: componentDir,
componentsPath: componentName,
injectClient: client,
},
wantErr: false,
},
{
name: "valid url without `download`, with version",
field: field{
providerConfig: config.NewProvider("test", fmt.Sprintf("https://github.com/%s/%s/releases/%s/%s", owner, repo, version, path.Join(componentDir, componentName)), clusterctlv1.CoreProviderType),
variableClient: test.NewFakeVariableClient(),
},
want: &gitHubRepository{
providerConfig: config.NewProvider("test", fmt.Sprintf("https://github.com/%s/%s/releases/%s/%s", owner, repo, version, path.Join(componentDir, componentName)), clusterctlv1.CoreProviderType),
configVariablesClient: test.NewFakeVariableClient(),
authenticatingHTTPClient: nil,
owner: owner,
repository: repo,
defaultVersion: version,
rootPath: componentDir,
componentsPath: componentName,
injectClient: client,
},
wantErr: false,
},
{
name: "provider url should be in https://github.com/{owner}/{Repository}/releases/download/{version}/{componentsClient.yaml} format",
field: field{
providerConfig: config.NewProvider("test", "https://github.com/o/r1/releases/v1.2.3/download/path", clusterctlv1.CoreProviderType),
variableClient: test.NewFakeVariableClient(),
},
want: nil,
wantErr: true,
},
{
name: "provider url should be in https://github.com/{owner}/{Repository}/releases/latest/download/{componentsClient.yaml} format",
field: field{
providerConfig: config.NewProvider("test", "https://github.com/o/r1/releases/download/latest/path", clusterctlv1.CoreProviderType),
variableClient: test.NewFakeVariableClient(),
},
want: nil,
wantErr: true,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
g := NewWithT(t)
resetCaches()

gitHub, err := NewGitHubRepository(context.Background(), tt.field.providerConfig, tt.field.variableClient)
gitHub, err := NewGitHubRepository(context.Background(), tt.field.providerConfig, tt.field.variableClient, injectGithubClient(client))
if tt.wantErr {
g.Expect(err).To(HaveOccurred())
return
Expand Down
Loading