Skip to content

Commit

Permalink
Remove github_host var, rename github_token to github_access_token, m…
Browse files Browse the repository at this point in the history
…atch the release sources to the release by org
  • Loading branch information
jajita committed Oct 18, 2024
1 parent 6f3b66e commit 96e337d
Show file tree
Hide file tree
Showing 12 changed files with 138 additions and 131 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ Feature: As a robot, I want to generate release notes
| --name=hello-release |
| --version=v0.1.5 |
| --without-download |
| --variable=github_token="${GITHUB_TOKEN}" |
| --variable=github_access_token="${GITHUB_TOKEN}" |
And I write file "version"
| 0.1.4 |
And I execute git add Kilnfile.lock version
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ func githubRepoHasReleaseWithTag(ctx context.Context, repoOrg, repoName, tag str
if err != nil {
return err
}
ghAPI, err := gh.Client(ctx, "", accessToken)
ghAPI, err := gh.Client(ctx, "", accessToken, accessToken)
if err != nil {
return fmt.Errorf("failed to setup github client: %w", err)
}
Expand Down
71 changes: 34 additions & 37 deletions internal/commands/release_notes.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,15 @@ const releaseDateFormat = "2006-01-02"

type ReleaseNotes struct {
Options struct {
ReleaseDate string `long:"release-date" short:"d" description:"release date of the tile"`
TemplateName string `long:"template" short:"t" description:"path to template"`
GithubIssuesServiceToken string `long:"github-token" short:"g" description:"auth token for fetching issues and milestones with labels" env:"GITHUB_TOKEN"`
GithubHost string `long:"github-host" description:"set this when you are using GitHub enterprise to fetch issues, milestones or notes for bosh releases" env:"GITHUB_HOST"`
Kilnfile string `long:"kilnfile" short:"k" description:"path to Kilnfile"`
DocsFile string `long:"update-docs" short:"u" description:"path to docs file to update"`
Window string `long:"window" short:"w" description:"GA window for release notes" default:"ga"`
VariableFiles []string `long:"variables-file" short:"vf" description:"path to a file containing variables to interpolate"`
Variables []string `long:"variable" short:"vr" description:"key value pairs of variables to interpolate"`
ReleaseDate string `long:"release-date" short:"d" description:"release date of the tile"`
TemplateName string `long:"template" short:"t" description:"path to template"`
GithubAccessToken string `long:"github_access_token" short:"g" description:"auth token for github.com" env:"GITHUB_ACCESS_TOKEN"`
GithubEnterpriseAccessToken string `long:"github_enterprise_access_token" short:"ge" description:"auth token for github enterprise" env:"GITHUB_ENTERPRISE_ACCESS_TOKEN"`
Kilnfile string `long:"kilnfile" short:"k" description:"path to Kilnfile"`
DocsFile string `long:"update-docs" short:"u" description:"path to docs file to update"`
Window string `long:"window" short:"w" description:"GA window for release notes" default:"ga"`
VariableFiles []string `long:"variables-file" short:"vf" description:"path to a file containing variables to interpolate"`
Variables []string `long:"variable" short:"vr" description:"key value pairs of variables to interpolate"`
notes.IssuesQuery
notes.TrainstatQuery
}
Expand All @@ -49,10 +49,10 @@ type ReleaseNotes struct {
fetchNotesData FetchNotesData
variablesService baking.TemplateVariablesService

repoOwner, repoName string
repoHost, repoOwner, repoName string
}

type FetchNotesData func(ctx context.Context, repo *git.Repository, client *github.Client, tileRepoOwner, tileRepoName, kilnfilePath, initialRevision, finalRevision string, issuesQuery notes.IssuesQuery, trainstatClient notes.TrainstatNotesFetcher, variables map[string]any) (notes.Data, error)
type FetchNotesData func(ctx context.Context, repo *git.Repository, client *github.Client, tileRepoHost, tileRepoOwner, tileRepoName, kilnfilePath, initialRevision, finalRevision string, issuesQuery notes.IssuesQuery, trainstatClient notes.TrainstatNotesFetcher, variables map[string]any) (notes.Data, error)

func NewReleaseNotesCommand() (ReleaseNotes, error) {
return ReleaseNotes{
Expand Down Expand Up @@ -82,16 +82,16 @@ func (r ReleaseNotes) Execute(args []string) error {
if err != nil {
return fmt.Errorf("failed to parse template variables: %s", err)
}
if varValue, ok := templateVariables["github_token"]; !ok && r.Options.GithubIssuesServiceToken != "" {
templateVariables["github_token"] = r.Options.GithubIssuesServiceToken
} else if ok && r.Options.GithubIssuesServiceToken == "" {
r.Options.GithubIssuesServiceToken = varValue.(string)
if varValue, ok := templateVariables["github_access_token"]; !ok && r.Options.GithubAccessToken != "" {
templateVariables["github_access_token"] = r.Options.GithubAccessToken
} else if ok && r.Options.GithubAccessToken == "" {
r.Options.GithubAccessToken = varValue.(string)
}

if varValue, ok := templateVariables["github_host"]; !ok && r.Options.GithubHost != "" {
templateVariables["github_host"] = r.Options.GithubHost
} else if ok && r.Options.GithubHost == "" {
r.Options.GithubHost = varValue.(string)
if varValue, ok := templateVariables["github_enterprise_access_token"]; !ok && r.Options.GithubAccessToken != "" {
templateVariables["github_enterprise_access_token"] = r.Options.GithubEnterpriseAccessToken
} else if ok && r.Options.GithubEnterpriseAccessToken == "" {
r.Options.GithubEnterpriseAccessToken = varValue.(string)
}

ctx := context.Background()
Expand All @@ -106,18 +106,17 @@ func (r ReleaseNotes) Execute(args []string) error {
}

var client *github.Client
if r.Options.GithubIssuesServiceToken != "" {
client, err = gh.Client(ctx, r.Options.GithubHost, r.Options.GithubIssuesServiceToken)
if err != nil {
return fmt.Errorf("failed to setup github client: %w", err)
}

client, err = gh.Client(ctx, r.repoHost, r.Options.GithubAccessToken, r.Options.GithubEnterpriseAccessToken)
if err != nil {
return fmt.Errorf("failed to setup github client: %w", err)
}

trainstatClient := notes.NewTrainstatClient(r.Options.TrainstatQuery.TrainstatURL)

_ = notes.FetchData // fetchNotesData is github.com/pivotal/kiln/internal/notes.FetchData
data, err := r.fetchNotesData(ctx,
r.repository, client, r.repoOwner, r.repoName,
r.repository, client, r.repoHost, r.repoOwner, r.repoName,
r.Options.Kilnfile,
nonFlagArgs[0], nonFlagArgs[1],
r.Options.IssuesQuery,
Expand Down Expand Up @@ -196,12 +195,13 @@ func (r *ReleaseNotes) initRepo() error {
return fmt.Errorf("release-notes must be run from the root of the repository (use --kilnfile flag to specify which tile to build)")
}

repoOwner, repoName, err := getGithubRemoteRepoOwnerAndName(repo)
repoHost, repoOwner, repoName, err := getGithubRemoteHostRepoOwnerAndName(repo)
if err != nil {
return err
}

r.repository = repo
r.repoHost = repoHost
r.repoName = repoName
r.repoOwner = repoOwner

Expand Down Expand Up @@ -243,11 +243,8 @@ func (r ReleaseNotes) checkInputs(nonFlagArgs []string) error {
}
}

if r.Options.GithubIssuesServiceToken == "" &&
(r.Options.IssueMilestone != "" ||
len(r.Options.IssueIDs) > 0 ||
len(r.Options.IssueLabels) > 0) {
return errors.New("github-token (env: GITHUB_TOKEN) must be set to interact with the github api")
if r.Options.GithubEnterpriseAccessToken == "" && r.Options.GithubAccessToken == "" {
return errors.New("github_access_token(env: GITHUB_ACCESS_TOKEN) and/or github_enterprise_access_token(env: GITHUB_ENTERPRISE_ACCESS_TOKEN) must be set to interact with the github api")
}

if r.Options.DocsFile != "" {
Expand Down Expand Up @@ -279,25 +276,25 @@ func (r ReleaseNotes) parseReleaseDate() (time.Time, error) {
return releaseDate, nil
}

func getGithubRemoteRepoOwnerAndName(repo *git.Repository) (string, string, error) {
func getGithubRemoteHostRepoOwnerAndName(repo *git.Repository) (string, string, string, error) {
var remoteURL string
remote, err := repo.Remote("origin")
if err != nil {
return "", "", err
return "", "", "", err
}
config := remote.Config()
for _, u := range config.URLs {
remoteURL = u
break
}
if remoteURL == "" {
return "", "", fmt.Errorf("remote github URL not found for repo")
return "", "", "", fmt.Errorf("remote github URL not found for repo")
}

repoOwner, repoName, err := gh.RepositoryOwnerAndNameFromPath(remoteURL)
repoHost, repoOwner, repoName, err := gh.RepositoryHostOwnerAndNameFromPath(remoteURL)
if err != nil {
return "", "", err
return "", "", "", err
}

return repoOwner, repoName, nil
return repoHost, repoOwner, repoName, nil
}
47 changes: 18 additions & 29 deletions internal/commands/release_notes_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ func TestReleaseNotes_Execute(t *testing.T) {
}

var (
tileRepoOwner, tileRepoName, kilnfilePath, initialRevision, finalRevision string
tileRepoHost, tileRepoOwner, tileRepoName, kilnfilePath, initialRevision, finalRevision string

issuesQuery notes.IssuesQuery
repository *git.Repository
Expand All @@ -69,12 +69,13 @@ func TestReleaseNotes_Execute(t *testing.T) {
rn := ReleaseNotes{
Writer: &out,
repository: nonNilRepo,
repoHost: "github.com",
repoOwner: "bunch",
repoName: "banana",
readFile: readFileFunc,
fetchNotesData: func(c context.Context, repo *git.Repository, ghc *github.Client, tro, trn, kfp, ir, fr string, iq notes.IssuesQuery, _ notes.TrainstatNotesFetcher, __ map[string]any) (notes.Data, error) {
fetchNotesData: func(c context.Context, repo *git.Repository, ghc *github.Client, trh, tro, trn, kfp, ir, fr string, iq notes.IssuesQuery, _ notes.TrainstatNotesFetcher, __ map[string]any) (notes.Data, error) {
ctx, repository, client = c, repo, ghc
tileRepoOwner, tileRepoName, kilnfilePath, initialRevision, finalRevision = tro, trn, kfp, ir, fr
tileRepoHost, tileRepoOwner, tileRepoName, kilnfilePath, initialRevision, finalRevision = trh, tro, trn, kfp, ir, fr
issuesQuery = iq
return notes.Data{
ReleaseDate: mustParseTime(time.Parse(releaseDateFormat, "2021-11-04")),
Expand Down Expand Up @@ -104,12 +105,12 @@ func TestReleaseNotes_Execute(t *testing.T) {
},
}

rn.Options.GithubIssuesServiceToken = "secret"
rn.Options.GithubAccessToken = "secret"

err := rn.Execute([]string{
"--kilnfile=tile/Kilnfile",
"--release-date=2021-11-04",
"--github-token=lemon",
"--github_access_token=lemon",
"--github-issue-milestone=smoothie",
"--github-issue-label=tropical",
"--github-issue=54000",
Expand All @@ -124,6 +125,7 @@ func TestReleaseNotes_Execute(t *testing.T) {
please.Expect(repository).NotTo(BeNil())
please.Expect(client).NotTo(BeNil())

please.Expect(tileRepoHost).To(Equal("github.com"))
please.Expect(tileRepoOwner).To(Equal("bunch"))
please.Expect(tileRepoName).To(Equal("banana"))
please.Expect(kilnfilePath).To(Equal("tile/Kilnfile"))
Expand Down Expand Up @@ -188,6 +190,7 @@ func TestReleaseNotes_checkInputs(t *testing.T) {

rn := ReleaseNotes{}
rn.Options.ReleaseDate = `some-date`
rn.Options.GithubAccessToken = "test-token"
err := rn.checkInputs([]string{"a", "b"})
please.Expect(err).To(MatchError(ContainSubstring("cannot parse")))
})
Expand All @@ -197,34 +200,17 @@ func TestReleaseNotes_checkInputs(t *testing.T) {
please := NewWithT(t)

rn := ReleaseNotes{}
rn.Options.IssueMilestone = "s"
err := rn.checkInputs([]string{"a", "b"})
please.Expect(err).To(MatchError(ContainSubstring("github-token")))
})

t.Run("ids", func(t *testing.T) {
please := NewWithT(t)

rn := ReleaseNotes{}
rn.Options.IssueIDs = []string{"s"}
err := rn.checkInputs([]string{"a", "b"})
please.Expect(err).To(MatchError(ContainSubstring("github-token")))
})

t.Run("labels", func(t *testing.T) {
please := NewWithT(t)

rn := ReleaseNotes{}
rn.Options.IssueLabels = []string{"s"}
err := rn.checkInputs([]string{"a", "b"})
please.Expect(err).To(MatchError(ContainSubstring("github-token")))
please.Expect(err).To(MatchError(ContainSubstring("github_access_token")))
please.Expect(err).To(MatchError(ContainSubstring("github_enterprise_access_token")))
})

t.Run("exp", func(t *testing.T) {
please := NewWithT(t)

rn := ReleaseNotes{}
rn.Options.IssueTitleExp = "s"
rn.Options.GithubEnterpriseAccessToken = "test-token"
err := rn.checkInputs([]string{"a", "b"})
please.Expect(err).NotTo(HaveOccurred())
})
Expand All @@ -243,8 +229,9 @@ func Test_getGithubRemoteRepoOwnerAndName(t *testing.T) {
"https://github.com/pivotal-cf/kiln",
},
})
o, r, err := getGithubRemoteRepoOwnerAndName(repo)
h, o, r, err := getGithubRemoteHostRepoOwnerAndName(repo)
please.Expect(err).NotTo(HaveOccurred())
please.Expect(h).To(Equal("github.com"))
please.Expect(o).To(Equal("pivotal-cf"))
please.Expect(r).To(Equal("kiln"))
})
Expand All @@ -259,8 +246,9 @@ func Test_getGithubRemoteRepoOwnerAndName(t *testing.T) {
"git@github.com:pivotal-cf/kiln.git",
},
})
o, r, err := getGithubRemoteRepoOwnerAndName(repo)
h, o, r, err := getGithubRemoteHostRepoOwnerAndName(repo)
please.Expect(err).NotTo(HaveOccurred())
please.Expect(h).To(Equal("github.com"))
please.Expect(o).To(Equal("pivotal-cf"))
please.Expect(r).To(Equal("kiln"))
})
Expand All @@ -269,7 +257,7 @@ func Test_getGithubRemoteRepoOwnerAndName(t *testing.T) {
please := NewWithT(t)

repo, _ := git.Init(memory.NewStorage(), memfs.New())
_, _, err := getGithubRemoteRepoOwnerAndName(repo)
_, _, _, err := getGithubRemoteHostRepoOwnerAndName(repo)
please.Expect(err).To(MatchError(ContainSubstring("not found")))
})

Expand All @@ -289,8 +277,9 @@ func Test_getGithubRemoteRepoOwnerAndName(t *testing.T) {
"git@github.com:pivotal-cf/kiln.git",
},
})
o, _, err := getGithubRemoteRepoOwnerAndName(repo)
h, o, _, err := getGithubRemoteHostRepoOwnerAndName(repo)
please.Expect(err).NotTo(HaveOccurred())
please.Expect(h).To(Equal("github.com"))
please.Expect(o).To(Equal("pivotal-cf"), "it uses the remote with name 'origin'")
})
}
Expand Down
2 changes: 1 addition & 1 deletion internal/component/github_release_source.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ func NewGithubReleaseSource(c cargo.ReleaseSourceConfig) *GithubReleaseSource {
if c.Org == "" {
panic("no github org passed for github release source")
}
githubClient, err := c.GitHubClient(context.TODO())
githubClient, err := gh.Client(context.TODO(), "", c.GithubToken, c.GithubToken) // host is github.com by default
if err != nil {
panic(err)
}
Expand Down
26 changes: 22 additions & 4 deletions internal/gh/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,33 @@ package gh

import (
"context"
"errors"
"fmt"
"strings"

"github.com/google/go-github/v50/github"
"golang.org/x/oauth2"
)

func Client(ctx context.Context, host, accessToken string) (*github.Client, error) {
client := oauth2.NewClient(ctx, oauth2.StaticTokenSource(&oauth2.Token{AccessToken: accessToken}))
if host == "" {
/* Client
Creates a GitHub client based on the host.
If host = GitHub Enterprise Host, uses githubEnterpriseAccessToken
Else it assumes host as GitHub.com and uses githubAccessToken
*/

func Client(ctx context.Context, host, githubAccessToken string, githubEnterpriseAccessToken string) (*github.Client, error) {
if host != "" && strings.HasSuffix(host, "broadcom.net") {
if githubEnterpriseAccessToken == "" {
return nil, errors.New("github enterprise access token is absent")
}
client := oauth2.NewClient(ctx, oauth2.StaticTokenSource(&oauth2.Token{AccessToken: githubEnterpriseAccessToken}))
return github.NewEnterpriseClient(host, host, client)
} else if host == "" || host == "github.com" {
if githubAccessToken == "" {
return nil, fmt.Errorf("github access token (github.com) is absent")
}
client := oauth2.NewClient(ctx, oauth2.StaticTokenSource(&oauth2.Token{AccessToken: githubAccessToken}))
return github.NewClient(client), nil
}
return github.NewEnterpriseClient(host, host, client)
return nil, errors.New("github host not recognized")
}
16 changes: 12 additions & 4 deletions internal/gh/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,18 +13,26 @@ func TestClient(t *testing.T) {
t.Run("when the host is empty", func(t *testing.T) {
ctx := context.Background()
token := "xxx"
ghClient, err := gh.Client(ctx, "", token)
ghClient, err := gh.Client(ctx, "", token, token)
require.NoError(t, err)
require.NotNil(t, ghClient.Client())
assert.Contains(t, ghClient.BaseURL.String(), "https://api.github.com")
})

t.Run("when the host is not empty", func(t *testing.T) {
t.Run("when the host point to enterprise github", func(t *testing.T) {
ctx := context.Background()
token := "xxx"
ghClient, err := gh.Client(ctx, "https://example.com", token)
ghClient, err := gh.Client(ctx, "https://broadcom.net", token, token)
require.NoError(t, err)
require.NotNil(t, ghClient.Client())
assert.Contains(t, ghClient.BaseURL.String(), "https://example.com")
assert.Contains(t, ghClient.BaseURL.String(), "https://broadcom.net")
})

t.Run("when the host point to non-enterprise random github", func(t *testing.T) {
ctx := context.Background()
token := "xxx"
ghClient, err := gh.Client(ctx, "https://example.com", token, token)
require.Error(t, err, "github host not recognized")
require.Nil(t, ghClient)
})
}
Loading

0 comments on commit 96e337d

Please sign in to comment.