Skip to content

Commit dca4917

Browse files
committed
Allow github:ORG// repos to redirect to other repos in the same org
Signed-off-by: Jan Dubois <[email protected]>
1 parent 8944d8b commit dca4917

File tree

3 files changed

+194
-59
lines changed

3 files changed

+194
-59
lines changed

hack/bats/tests/url-github.bats

Lines changed: 84 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -6,19 +6,33 @@ load "../helpers/load"
66
# The jandubois/jandubois GitHub repo has been especially constructed to test
77
# various features of the github URL scheme:
88
#
9-
# * repo defaults to org when not specified
10-
# * filename defaults to .lima.yaml when only a path is specified
11-
# * .yaml default extension
12-
# * .lima.yaml files may be treated as symlinks
13-
# * default branch lookup when not specified
9+
# * repo defaults to org when not specified
10+
# * filename defaults to .lima.yaml when only a path is specified
11+
# * .yaml default extension
12+
# * .lima.yaml files may be treated as symlinks
13+
# * default branch lookup when not specified
14+
# * github:ORG// repos can redirect to another github:ORG URL in the same ORG
1415
#
15-
# The repo files are:
16+
# The jandubois/jandubois repo files are:
1617
#
17-
# ├── .lima.yaml -> templates/demo.yaml
18-
# ├── docs
19-
# │ └── .lima.yaml -> ../templates/demo.yaml
20-
# └── templates
21-
# └── demo.yaml
18+
# ├── .lima.yaml -> templates/demo.yaml
19+
# ├── back
20+
# │ └── .lima.yaml -> github:jandubois//loop/
21+
# ├── docs
22+
# │ └── .lima.yaml -> ../templates/demo.yaml
23+
# ├── invalid
24+
# │ ├── org
25+
# │ │ └── .lima.yaml -> github:lima-vm
26+
# │ └── tag
27+
# │ └── .lima.yaml -> github:jandubois//@v0.0.0
28+
# ├── loop
29+
# │ └── .lima.yaml -> github:jandubois//back/
30+
# ├── redirect
31+
# │ └── .lima.yaml -> github:jandubois/lima/templates/default
32+
# ├── templates
33+
# │ └── demo.yaml "base: template:default"
34+
# └── yaml
35+
# └── .lima.yaml "{}"
2236
#
2337
# Both the `main` branch and the `v0.0.0` tag have this layout.
2438

@@ -52,7 +66,7 @@ URLS=(
5266
)
5367

5468
url() {
55-
run_e "$1" limactl template url "$2"
69+
run_e "$1" limactl --debug template url "$2"
5670
}
5771

5872
test_jandubois_url() {
@@ -71,9 +85,14 @@ for url in "${URLS[@]}"; do
7185
bats_test_function --description "$url" -- test_jandubois_url "$url"
7286
done
7387

74-
@test '.lima.yaml is retained when it is not a symlink' {
75-
url -0 'github:jandubois//test/'
76-
assert_output 'https://raw.githubusercontent.com/jandubois/jandubois/main/test/.lima.yaml'
88+
@test '.lima.yaml is retained when it exits and is not a symlink' {
89+
url -0 'github:jandubois//yaml/'
90+
assert_output 'https://raw.githubusercontent.com/jandubois/jandubois/main/yaml/.lima.yaml'
91+
}
92+
93+
@test 'non-existing .lima.yaml returns an error' {
94+
url -1 'github:jandubois//missing/'
95+
assert_fatal 'file "https://raw.githubusercontent.com/jandubois/jandubois/main/missing/.lima.yaml" not found or inaccessible: status 404'
7796
}
7897

7998
@test 'hidden files without an extension get a .yaml extension' {
@@ -88,15 +107,62 @@ done
88107

89108
@test 'github: URLs are EXPERIMENTAL' {
90109
url -0 'github:jandubois'
91-
assert_stderr --regexp 'warning.+GitHub locator .* replaced with .* EXPERIMENTAL'
110+
assert_warning "The github: scheme is still EXPERIMENTAL"
92111
}
93112

94-
@test 'Empty github: url returns an error' {
113+
# Invalid URLs
114+
@test 'empty github: url returns an error' {
95115
url -1 'github:'
96116
assert_fatal 'github: URL must contain at least an ORG, got ""'
97117
}
98118

99-
@test 'Missing org returns an error' {
119+
@test 'missing org returns an error' {
100120
url -1 'github:/jandubois'
101121
assert_fatal 'github: URL must contain at least an ORG, got ""'
102122
}
123+
124+
# github: redirects in github:ORG// repos
125+
@test 'org redirects can point to different repo and may switch the branch name' {
126+
url -0 'github:jandubois//redirect/'
127+
# Note that the default branch in jandubois/jandubois is main, but in jandubois/lima it is master
128+
assert_debug 'Locator "github:jandubois//redirect/" replaced with "github:jandubois/lima/templates/default"'
129+
assert_debug 'Locator "github:jandubois/lima/templates/default" replaced with "https://raw.githubusercontent.com/jandubois/lima/master/templates/default.yaml"'
130+
assert_output 'https://raw.githubusercontent.com/jandubois/lima/master/templates/default.yaml'
131+
}
132+
133+
@test 'org redirects propagate an explicit branch/tag to the other repo' {
134+
url -0 'github:jandubois//redirect/@v1.2.1'
135+
assert_debug 'Locator "github:jandubois//redirect/@v1.2.1" replaced with "github:jandubois/lima/templates/[email protected]"'
136+
assert_debug 'Locator "github:jandubois/lima/templates/[email protected]" replaced with "https://raw.githubusercontent.com/jandubois/lima/v1.2.1/templates/default.yaml"'
137+
assert_output 'https://raw.githubusercontent.com/jandubois/lima/v1.2.1/templates/default.yaml'
138+
}
139+
140+
@test 'org redirects cannot point to another org' {
141+
url -1 'github:jandubois//invalid/org/'
142+
assert_fatal 'redirect "github:lima-vm" is not a "github:jandubois" URL…'
143+
}
144+
145+
@test 'org redirects with branch cannot point to another org' {
146+
url -1 'github:jandubois//invalid/org/@main'
147+
assert_fatal 'redirect "github:lima-vm" is not a "github:jandubois" URL…'
148+
}
149+
150+
@test 'org redirects cannot include a branch or tag' {
151+
url -1 'github:jandubois//invalid/tag/'
152+
assert_fatal 'redirect "github:jandubois//@v0.0.0" must not include a branch/tag/sha…'
153+
}
154+
155+
@test 'org redirects with tag cannot include a branch or tag' {
156+
url -1 'github:jandubois//invalid/tag/@v0.0.0'
157+
assert_fatal 'redirect "github:jandubois//@v0.0.0" must not include a branch/tag/sha…'
158+
}
159+
160+
@test 'org redirects must not create circular redirects' {
161+
url -1 'github:jandubois//loop/'
162+
assert_fatal 'custom locator "github:jandubois//loop/" has a redirect loop'
163+
}
164+
165+
@test 'org redirects with branch must not create circular redirects' {
166+
url -1 'github:jandubois//back/@main'
167+
assert_fatal 'custom locator "github:jandubois//back/@main" has a redirect loop'
168+
}

pkg/limatmpl/github.go

Lines changed: 83 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@ import (
1616
"strings"
1717
)
1818

19+
const defaultFilename = ".lima.yaml"
20+
1921
// transformGitHubURL transforms a github: URL to a raw.githubusercontent.com URL.
2022
// Input format: ORG/REPO[/PATH][@BRANCH]
2123
//
@@ -25,12 +27,7 @@ import (
2527
// If PATH is just a directory (trailing slash), it will be set to .lima.yaml
2628
// IF FILE is .lima.yaml and contents looks like a symlink, it will be replaced by the symlink target.
2729
func transformGitHubURL(ctx context.Context, input string) (string, error) {
28-
// Check for explicit branch specification with @ at the end
29-
var branch string
30-
if idx := strings.LastIndex(input, "@"); idx != -1 {
31-
branch = input[idx+1:]
32-
input = input[:idx]
33-
}
30+
input, origBranch, _ := strings.Cut(input, "@")
3431

3532
parts := strings.Split(input, "/")
3633
for len(parts) < 2 {
@@ -44,24 +41,25 @@ func transformGitHubURL(ctx context.Context, input string) (string, error) {
4441

4542
// If REPO is omitted (github:ORG or github:ORG//PATH), default it to ORG
4643
repo := cmp.Or(parts[1], org)
47-
pathPart := strings.Join(parts[2:], "/")
44+
filePath := strings.Join(parts[2:], "/")
4845

49-
if pathPart == "" {
50-
pathPart = ".lima.yaml"
46+
if filePath == "" {
47+
filePath = defaultFilename
5148
} else {
5249
// If path ends with /, it's a directory, so append .lima
53-
if strings.HasSuffix(pathPart, "/") {
54-
pathPart += ".lima"
50+
if strings.HasSuffix(filePath, "/") {
51+
filePath += defaultFilename
5552
}
5653

5754
// If the filename (excluding first char for hidden files) has no extension, add .yaml
58-
filename := path.Base(pathPart)
55+
filename := path.Base(filePath)
5956
if !strings.Contains(filename[1:], ".") {
60-
pathPart += ".yaml"
57+
filePath += ".yaml"
6158
}
6259
}
6360

6461
// Query default branch if no branch was specified
62+
branch := origBranch
6563
if branch == "" {
6664
var err error
6765
branch, err = getGitHubDefaultBranch(ctx, org, repo)
@@ -71,13 +69,24 @@ func transformGitHubURL(ctx context.Context, input string) (string, error) {
7169
}
7270

7371
// If filename is .lima.yaml, check if it's a symlink/redirect to another file
74-
if path.Base(pathPart) == ".lima.yaml" {
75-
if redirectPath, err := resolveGitHubSymlink(ctx, org, repo, branch, pathPart); err == nil {
76-
pathPart = redirectPath
77-
}
72+
if path.Base(filePath) == defaultFilename {
73+
return resolveGitHubSymlink(ctx, org, repo, branch, filePath, origBranch)
7874
}
75+
return githubUserContentURL(org, repo, branch, filePath), nil
76+
}
77+
78+
func githubUserContentURL(org, repo, branch, filePath string) string {
79+
return fmt.Sprintf("https://raw.githubusercontent.com/%s/%s/%s/%s", org, repo, branch, filePath)
80+
}
7981

80-
return fmt.Sprintf("https://raw.githubusercontent.com/%s/%s/%s/%s", org, repo, branch, pathPart), nil
82+
func getGitHubUserContent(ctx context.Context, org, repo, branch, filePath string) (*http.Response, error) {
83+
url := githubUserContentURL(org, repo, branch, filePath)
84+
req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, http.NoBody)
85+
if err != nil {
86+
return nil, fmt.Errorf("failed to create request: %w", err)
87+
}
88+
req.Header.Set("User-Agent", "lima")
89+
return http.DefaultClient.Do(req)
8190
}
8291

8392
// getGitHubDefaultBranch queries the GitHub API to get the default branch for a repository.
@@ -129,40 +138,79 @@ func getGitHubDefaultBranch(ctx context.Context, org, repo string) (string, erro
129138
}
130139

131140
// resolveGitHubSymlink checks if a file at the given path is a symlink/redirect to another file.
132-
// If the file contains a single line without YAML content, it's treated as a path to the actual file.
133-
// Returns the redirect path if found, or the original path otherwise.
134-
func resolveGitHubSymlink(ctx context.Context, org, repo, branch, filePath string) (string, error) {
135-
url := fmt.Sprintf("https://raw.githubusercontent.com/%s/%s/%s/%s", org, repo, branch, filePath)
136-
137-
req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, http.NoBody)
138-
if err != nil {
139-
return "", fmt.Errorf("failed to create request: %w", err)
140-
}
141-
142-
req.Header.Set("User-Agent", "lima")
143-
144-
resp, err := http.DefaultClient.Do(req)
141+
// If the file contains a single line without newline, space, or colon then it's treated as a path to the actual file.
142+
// Returns a URL to the redirect path if found, or a URL for original path otherwise.
143+
func resolveGitHubSymlink(ctx context.Context, org, repo, branch, filePath, origBranch string) (string, error) {
144+
resp, err := getGitHubUserContent(ctx, org, repo, branch, filePath)
145145
if err != nil {
146146
return "", fmt.Errorf("failed to fetch file: %w", err)
147147
}
148148
defer resp.Body.Close()
149149

150+
// Special rule for branch/tag propagation for github:ORG// requests.
151+
if resp.StatusCode == http.StatusNotFound && repo == org {
152+
defaultBranch, err := getGitHubDefaultBranch(ctx, org, repo)
153+
if err == nil {
154+
return resolveGitHubRedirect(ctx, org, repo, defaultBranch, filePath, branch)
155+
}
156+
}
150157
if resp.StatusCode != http.StatusOK {
151-
return "", fmt.Errorf("file not found or inaccessible: status %d", resp.StatusCode)
158+
return "", fmt.Errorf("file %q not found or inaccessible: status %d", resp.Request.URL, resp.StatusCode)
152159
}
153160

154161
// Read first 1KB to check the file content
155162
buf := make([]byte, 1024)
156163
n, err := resp.Body.Read(buf)
157164
if err != nil && !errors.Is(err, io.EOF) {
158-
return "", fmt.Errorf("failed to read file content: %w", err)
165+
return "", fmt.Errorf("failed to read %q content: %w", resp.Request.URL, err)
159166
}
160167
content := string(buf[:n])
161168

169+
// Symlink can also be a github: redirect if we are in a github:ORG// repo.
170+
if repo == org && strings.HasPrefix(content, "github:") {
171+
return validateGitHubRedirect(content, org, origBranch, resp.Request.URL.String())
172+
}
173+
162174
// A symlink must be a single line (without trailing newline), no spaces, no colons
163175
if !(content == "" || strings.ContainsAny(content, "\n :")) {
164176
// symlink is relative to the directory of filePath
165-
return path.Join(path.Dir(filePath), content), nil
177+
filePath = path.Join(path.Dir(filePath), content)
178+
}
179+
return githubUserContentURL(org, repo, branch, filePath), nil
180+
}
181+
182+
// resolveGitHubRedirect checks if a file at the given path is a github: URL to another file within the same repo.
183+
// Returns the URL, or an error if the file doesn't exist, or doesn't start with github:ORG.
184+
func resolveGitHubRedirect(ctx context.Context, org, repo, defaultBranch, filePath, origBranch string) (string, error) {
185+
// Refetch the filepath from the defaultBranch
186+
resp, err := getGitHubUserContent(ctx, org, repo, defaultBranch, filePath)
187+
if err != nil {
188+
return "", fmt.Errorf("failed to fetch file: %w", err)
189+
}
190+
defer resp.Body.Close()
191+
if resp.StatusCode != http.StatusOK {
192+
return "", fmt.Errorf("file %q not found or inaccessible: status %d", resp.Request.URL, resp.StatusCode)
193+
}
194+
body, err := io.ReadAll(resp.Body)
195+
if err != nil {
196+
return "", fmt.Errorf("failed to read %q content: %w", resp.Request.URL, err)
197+
}
198+
return validateGitHubRedirect(string(body), org, origBranch, resp.Request.URL.String())
199+
}
200+
201+
func validateGitHubRedirect(body, org, origBranch, url string) (string, error) {
202+
redirect, _, _ := strings.Cut(body, "\n")
203+
redirect = strings.TrimSpace(redirect)
204+
205+
if !strings.HasPrefix(redirect, "github:"+org+"/") {
206+
return "", fmt.Errorf(`redirect %q is not a "github:%s" URL (from %q)`, redirect, org, url)
207+
}
208+
if strings.ContainsRune(redirect, '@') {
209+
return "", fmt.Errorf("redirect %q must not include a branch/tag/sha (from %q)", redirect, url)
210+
}
211+
// If the origBranch is empty, then we need to look up the default branch in the redirect
212+
if origBranch != "" {
213+
redirect += "@" + origBranch
166214
}
167-
return filePath, nil
215+
return redirect, nil
168216
}

pkg/limatmpl/locator.go

Lines changed: 27 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -296,7 +296,7 @@ func InstNameFromYAMLPath(yamlPath string) (string, error) {
296296
return s, nil
297297
}
298298

299-
func TransformCustomURL(ctx context.Context, locator string) (string, error) {
299+
func transformCustomURL(ctx context.Context, locator string) (string, error) {
300300
u, err := url.Parse(locator)
301301
if err != nil || len(u.Scheme) <= 1 {
302302
return locator, nil
@@ -317,7 +317,6 @@ func TransformCustomURL(ctx context.Context, locator string) (string, error) {
317317
if err != nil {
318318
return "", err
319319
}
320-
logrus.Warnf("GitHub locator %q replaced with %q is still EXPERIMENTAL", locator, newLocator)
321320
return newLocator, nil
322321
}
323322

@@ -350,9 +349,31 @@ func TransformCustomURL(ctx context.Context, locator string) (string, error) {
350349
}
351350
return "", fmt.Errorf("command %q failed: %w", cmd.String(), err)
352351
}
353-
newLocator := strings.TrimSpace(string(stdout))
354-
if newLocator != locator {
355-
logrus.Debugf("Custom locator %q replaced with %q", locator, newLocator)
352+
return strings.TrimSpace(string(stdout)), nil
353+
}
354+
355+
func TransformCustomURL(ctx context.Context, locator string) (string, error) {
356+
seen := make(map[string]bool)
357+
origLocator := locator
358+
githubSchemeDetected := false
359+
360+
for !seen[locator] {
361+
seen[locator] = true
362+
if strings.HasPrefix(locator, "github:") {
363+
githubSchemeDetected = true
364+
}
365+
newLocator, err := transformCustomURL(ctx, locator)
366+
if err != nil {
367+
return "", err
368+
}
369+
if newLocator == locator {
370+
if githubSchemeDetected {
371+
logrus.Warn("The github: scheme is still EXPERIMENTAL")
372+
}
373+
return newLocator, nil
374+
}
375+
logrus.Debugf("Locator %q replaced with %q", locator, newLocator)
376+
locator = newLocator
356377
}
357-
return newLocator, nil
378+
return "", fmt.Errorf("custom locator %q has a redirect loop", origLocator)
358379
}

0 commit comments

Comments
 (0)