feat(skills): support GHEC with data residency hosts
Widen ValidateSupportedHost to accept tenancy hosts (*.ghe.com) alongside github.com. GHEC with data residency uses these domains, and all skill subcommands (search, install, preview, publish, update) now allow them. GitHub Enterprise Server remains unsupported and is explicitly rejected with a clear error message. Also fix the lockfile writer to use the actual host when constructing SourceURL instead of hardcoding github.com. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
parent
3ad29588b8
commit
63262dce8b
11 changed files with 52 additions and 19 deletions
|
|
@ -76,7 +76,7 @@ func Install(opts *Options) (*Result, error) {
|
|||
return nil, fmt.Errorf("failed to install skill %q: %w", skill.InstallName(), err)
|
||||
}
|
||||
var warnings []string
|
||||
if err := lockfile.RecordInstall(skill.InstallName(), opts.Owner, opts.Repo, skill.Path+"/SKILL.md", skill.TreeSHA, opts.PinnedRef); err != nil {
|
||||
if err := lockfile.RecordInstall(opts.Host, skill.InstallName(), opts.Owner, opts.Repo, skill.Path+"/SKILL.md", skill.TreeSHA, opts.PinnedRef); err != nil {
|
||||
warnings = append(warnings, fmt.Sprintf("could not record install for %s: %v", skill.InstallName(), err))
|
||||
}
|
||||
return &Result{Installed: []string{skill.InstallName()}, Dir: targetDir, Warnings: warnings}, nil
|
||||
|
|
@ -129,7 +129,7 @@ func Install(opts *Options) (*Result, error) {
|
|||
}
|
||||
installed = append(installed, r.name)
|
||||
skill := opts.Skills[i]
|
||||
if err := lockfile.RecordInstall(skill.InstallName(), opts.Owner, opts.Repo, skill.Path+"/SKILL.md", skill.TreeSHA, opts.PinnedRef); err != nil {
|
||||
if err := lockfile.RecordInstall(opts.Host, skill.InstallName(), opts.Owner, opts.Repo, skill.Path+"/SKILL.md", skill.TreeSHA, opts.PinnedRef); err != nil {
|
||||
warnings = append(warnings, fmt.Sprintf("could not record install for %s: %v", skill.InstallName(), err))
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -10,6 +10,7 @@ import (
|
|||
"time"
|
||||
|
||||
"github.com/cli/cli/v2/internal/flock"
|
||||
"github.com/cli/cli/v2/internal/ghinstance"
|
||||
)
|
||||
|
||||
const (
|
||||
|
|
@ -93,7 +94,7 @@ func writeTo(f *os.File, lf *file) error {
|
|||
// RecordInstall adds or updates a skill entry in the lock file.
|
||||
// It uses a file-based lock to prevent concurrent read-modify-write races
|
||||
// when multiple install processes run simultaneously.
|
||||
func RecordInstall(skillName, owner, repo, skillPath, treeSHA, pinnedRef string) error {
|
||||
func RecordInstall(host, skillName, owner, repo, skillPath, treeSHA, pinnedRef string) error {
|
||||
lockPath, err := lockfilePath()
|
||||
if err != nil {
|
||||
return err
|
||||
|
|
@ -124,7 +125,7 @@ func RecordInstall(skillName, owner, repo, skillPath, treeSHA, pinnedRef string)
|
|||
f.Skills[skillName] = entry{
|
||||
Source: owner + "/" + repo,
|
||||
SourceType: "github",
|
||||
SourceURL: "https://github.com/" + owner + "/" + repo + ".git",
|
||||
SourceURL: ghinstance.HostPrefix(host) + owner + "/" + repo + ".git",
|
||||
SkillPath: skillPath,
|
||||
SkillFolderHash: treeSHA,
|
||||
InstalledAt: installedAt,
|
||||
|
|
|
|||
|
|
@ -24,6 +24,7 @@ func TestRecordInstall(t *testing.T) {
|
|||
tests := []struct {
|
||||
name string
|
||||
setup func(t *testing.T)
|
||||
host string
|
||||
skill string
|
||||
owner string
|
||||
repo string
|
||||
|
|
@ -35,6 +36,7 @@ func TestRecordInstall(t *testing.T) {
|
|||
}{
|
||||
{
|
||||
name: "fresh install creates lockfile",
|
||||
host: "github.com",
|
||||
skill: "code-review",
|
||||
owner: "monalisa",
|
||||
repo: "octocat-skills",
|
||||
|
|
@ -55,8 +57,25 @@ func TestRecordInstall(t *testing.T) {
|
|||
assert.Empty(t, e.PinnedRef)
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "tenancy host uses correct URL",
|
||||
host: "mycompany.ghe.com",
|
||||
skill: "code-review",
|
||||
owner: "monalisa",
|
||||
repo: "octocat-skills",
|
||||
skillPath: "skills/code-review/SKILL.md",
|
||||
treeSHA: "abc123",
|
||||
verify: func(t *testing.T, lockPath string) {
|
||||
t.Helper()
|
||||
f := readTestLockfile(t, lockPath)
|
||||
require.Contains(t, f.Skills, "code-review")
|
||||
e := f.Skills["code-review"]
|
||||
assert.Equal(t, "https://mycompany.ghe.com/monalisa/octocat-skills.git", e.SourceURL)
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "install with pinned ref",
|
||||
host: "github.com",
|
||||
skill: "pr-summary",
|
||||
owner: "hubot",
|
||||
repo: "skills-repo",
|
||||
|
|
@ -73,8 +92,9 @@ func TestRecordInstall(t *testing.T) {
|
|||
name: "multiple skills coexist",
|
||||
setup: func(t *testing.T) {
|
||||
t.Helper()
|
||||
require.NoError(t, RecordInstall("code-review", "monalisa", "octocat-skills", "skills/code-review/SKILL.md", "sha1", ""))
|
||||
require.NoError(t, RecordInstall("github.com", "code-review", "monalisa", "octocat-skills", "skills/code-review/SKILL.md", "sha1", ""))
|
||||
},
|
||||
host: "github.com",
|
||||
skill: "issue-triage",
|
||||
owner: "monalisa",
|
||||
repo: "octocat-skills",
|
||||
|
|
@ -107,6 +127,7 @@ func TestRecordInstall(t *testing.T) {
|
|||
require.NoError(t, err)
|
||||
t.Cleanup(unlock)
|
||||
},
|
||||
host: "github.com",
|
||||
skill: "code-review",
|
||||
owner: "monalisa",
|
||||
repo: "octocat-skills",
|
||||
|
|
@ -123,6 +144,7 @@ func TestRecordInstall(t *testing.T) {
|
|||
require.NoError(t, os.MkdirAll(filepath.Dir(lockPath), 0o755))
|
||||
require.NoError(t, os.WriteFile(lockPath, []byte("{invalid json"), 0o644))
|
||||
},
|
||||
host: "github.com",
|
||||
skill: "code-review",
|
||||
owner: "monalisa",
|
||||
repo: "octocat-skills",
|
||||
|
|
@ -145,6 +167,7 @@ func TestRecordInstall(t *testing.T) {
|
|||
data, _ := json.Marshal(file{Version: 999, Skills: map[string]entry{"old-skill": {}}})
|
||||
require.NoError(t, os.WriteFile(lockPath, data, 0o644))
|
||||
},
|
||||
host: "github.com",
|
||||
skill: "code-review",
|
||||
owner: "monalisa",
|
||||
repo: "octocat-skills",
|
||||
|
|
@ -166,7 +189,7 @@ func TestRecordInstall(t *testing.T) {
|
|||
tt.setup(t)
|
||||
}
|
||||
|
||||
err := RecordInstall(tt.skill, tt.owner, tt.repo, tt.skillPath, tt.treeSHA, tt.pinnedRef)
|
||||
err := RecordInstall(tt.host, tt.skill, tt.owner, tt.repo, tt.skillPath, tt.treeSHA, tt.pinnedRef)
|
||||
if tt.wantErr {
|
||||
require.Error(t, err)
|
||||
return
|
||||
|
|
@ -181,10 +204,10 @@ func TestRecordInstall(t *testing.T) {
|
|||
t.Run("update preserves InstalledAt and updates treeSHA", func(t *testing.T) {
|
||||
lockPath := setupTestHome(t)
|
||||
|
||||
require.NoError(t, RecordInstall("code-review", "monalisa", "octocat-skills", "skills/code-review/SKILL.md", "old-sha", ""))
|
||||
require.NoError(t, RecordInstall("github.com", "code-review", "monalisa", "octocat-skills", "skills/code-review/SKILL.md", "old-sha", ""))
|
||||
firstInstalledAt := readTestLockfile(t, lockPath).Skills["code-review"].InstalledAt
|
||||
|
||||
require.NoError(t, RecordInstall("code-review", "monalisa", "octocat-skills", "skills/code-review/SKILL.md", "new-sha", ""))
|
||||
require.NoError(t, RecordInstall("github.com", "code-review", "monalisa", "octocat-skills", "skills/code-review/SKILL.md", "new-sha", ""))
|
||||
entry := readTestLockfile(t, lockPath).Skills["code-review"]
|
||||
|
||||
assert.Equal(t, "new-sha", entry.SkillFolderHash, "treeSHA should be updated")
|
||||
|
|
|
|||
|
|
@ -4,6 +4,8 @@ import (
|
|||
"fmt"
|
||||
"strings"
|
||||
|
||||
ghauth "github.com/cli/go-gh/v2/pkg/auth"
|
||||
|
||||
"github.com/cli/cli/v2/internal/ghrepo"
|
||||
)
|
||||
|
||||
|
|
@ -48,16 +50,21 @@ func ParseMetadataRepo(meta map[string]interface{}) (ghrepo.Interface, bool, err
|
|||
return repo, true, nil
|
||||
}
|
||||
|
||||
// ValidateSupportedHost rejects hosts that are not supported in public preview.
|
||||
// ValidateSupportedHost rejects hosts that are not supported.
|
||||
// Supported hosts are github.com and GHEC with data residency (*.ghe.com).
|
||||
// GitHub Enterprise Server is not currently supported.
|
||||
func ValidateSupportedHost(host string) error {
|
||||
host = normalizeHost(host)
|
||||
if host == "" {
|
||||
return fmt.Errorf("could not determine repository host")
|
||||
}
|
||||
if host != SupportedHost {
|
||||
return fmt.Errorf("GitHub Skills currently supports only %s as a host; got %s", SupportedHost, host)
|
||||
if host == SupportedHost || ghauth.IsTenancy(host) {
|
||||
return nil
|
||||
}
|
||||
return nil
|
||||
if ghauth.IsEnterprise(host) {
|
||||
return fmt.Errorf("GitHub Skills does not currently support GitHub Enterprise Server; got %s", host)
|
||||
}
|
||||
return fmt.Errorf("unsupported host for GitHub Skills: %s", host)
|
||||
}
|
||||
|
||||
func normalizeHost(host string) string {
|
||||
|
|
|
|||
|
|
@ -72,5 +72,7 @@ func TestParseMetadataRepo(t *testing.T) {
|
|||
|
||||
func TestValidateSupportedHost(t *testing.T) {
|
||||
require.NoError(t, ValidateSupportedHost("github.com"))
|
||||
require.ErrorContains(t, ValidateSupportedHost("acme.ghes.com"), "supports only github.com")
|
||||
require.NoError(t, ValidateSupportedHost("mycompany.ghe.com"), "GHEC data residency tenancy hosts should be accepted")
|
||||
require.ErrorContains(t, ValidateSupportedHost("acme.ghes.com"), "does not currently support GitHub Enterprise Server")
|
||||
require.ErrorContains(t, ValidateSupportedHost("github.localhost"), "unsupported host")
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1177,7 +1177,7 @@ func TestInstallRun(t *testing.T) {
|
|||
SkillName: "git-commit",
|
||||
}
|
||||
},
|
||||
wantErr: "supports only github.com",
|
||||
wantErr: "does not currently support GitHub Enterprise Server",
|
||||
},
|
||||
{
|
||||
name: "select all skills in interactive prompt",
|
||||
|
|
|
|||
|
|
@ -403,7 +403,7 @@ func TestPreviewRun_UnsupportedHost(t *testing.T) {
|
|||
repo: ghrepo.NewWithHost("github", "awesome-copilot", "acme.ghes.com"),
|
||||
Telemetry: &telemetry.NoOpService{},
|
||||
})
|
||||
require.ErrorContains(t, err, "supports only github.com")
|
||||
require.ErrorContains(t, err, "does not currently support GitHub Enterprise Server")
|
||||
}
|
||||
|
||||
func TestPreviewRun_Interactive(t *testing.T) {
|
||||
|
|
|
|||
|
|
@ -968,7 +968,7 @@ func detectGitHubRemote(gitClient *git.Client, dir string) (*gitHubRemote, error
|
|||
}
|
||||
|
||||
// parseGitHubURL extracts owner/repo from a GitHub remote URL.
|
||||
// Only GitHub.com URLs are recognized.
|
||||
// Only github.com and GHEC data residency (*.ghe.com) URLs are recognized.
|
||||
func parseGitHubURL(rawURL string) (ghrepo.Interface, error) {
|
||||
u, err := git.ParseURL(rawURL)
|
||||
if err != nil {
|
||||
|
|
|
|||
|
|
@ -171,7 +171,7 @@ func TestPublishRun_UnsupportedHost(t *testing.T) {
|
|||
HttpClient: func() (*http.Client, error) { return nil, nil },
|
||||
host: "acme.ghes.com",
|
||||
})
|
||||
require.ErrorContains(t, err, "supports only github.com")
|
||||
require.ErrorContains(t, err, "does not currently support GitHub Enterprise Server")
|
||||
}
|
||||
|
||||
func TestPublishRun(t *testing.T) {
|
||||
|
|
|
|||
|
|
@ -33,7 +33,7 @@ func TestSearchRun_UnsupportedHost(t *testing.T) {
|
|||
HttpClient: func() (*http.Client, error) { return &http.Client{}, nil },
|
||||
Config: func() (gh.Config, error) { return cfg, nil },
|
||||
})
|
||||
require.ErrorContains(t, err, "supports only github.com")
|
||||
require.ErrorContains(t, err, "does not currently support GitHub Enterprise Server")
|
||||
}
|
||||
|
||||
func TestNewCmdSearch(t *testing.T) {
|
||||
|
|
|
|||
|
|
@ -171,7 +171,7 @@ func TestScanInstalledSkills(t *testing.T) {
|
|||
require.NoError(t, err)
|
||||
require.Len(t, skills, 1)
|
||||
require.Error(t, skills[0].metadataErr)
|
||||
assert.Contains(t, skills[0].metadataErr.Error(), "supports only github.com")
|
||||
assert.Contains(t, skills[0].metadataErr.Error(), "does not currently support GitHub Enterprise Server")
|
||||
},
|
||||
},
|
||||
{
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue