From b2ce0ba37a846a97de902e66017a50fbb9b2d93d Mon Sep 17 00:00:00 2001 From: Christopher Hunter Date: Fri, 16 Aug 2024 14:28:06 -0700 Subject: [PATCH] feat: use new Kilnfile to construct GitHub client Co-authored-by: Joe Eltgroth story: TPCF-26493 this is required for generating tile release notes --- internal/commands/release_notes.go | 43 ++++++++++---- internal/commands/release_notes_test.go | 4 +- internal/component/github_release_source.go | 19 ++---- pkg/cargo/bump.go | 61 ++++++++++++++----- pkg/cargo/bump_test.go | 66 +++++++++++++++------ pkg/cargo/files.go | 4 +- pkg/cargo/kilnfile.go | 17 ++++++ pkg/notes/notes_data.go | 35 +++++++---- pkg/notes/notes_data_test.go | 4 -- pkg/notes/notes_page_test.go | 44 +++++++------- 10 files changed, 194 insertions(+), 103 deletions(-) diff --git a/internal/commands/release_notes.go b/internal/commands/release_notes.go index a94342a80..90bf39dd1 100644 --- a/internal/commands/release_notes.go +++ b/internal/commands/release_notes.go @@ -15,10 +15,12 @@ import ( "text/template" "time" + "github.com/go-git/go-billy/v5/osfs" "github.com/go-git/go-git/v5" "github.com/google/go-github/v50/github" "github.com/pivotal-cf/jhanda" + "github.com/pivotal-cf/kiln/internal/baking" "github.com/pivotal-cf/kiln/internal/gh" "github.com/pivotal-cf/kiln/pkg/notes" ) @@ -27,13 +29,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"` - GithubToken string `long:"github-token" short:"g" description:"auth token for fetching issues merged between releases" env:"GITHUB_TOKEN"` - GithubHost string `long:"github-host" description:"set this when you are using GitHub enterprise" 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"` + ReleaseDate string `long:"release-date" short:"d" description:"release date of the tile"` + TemplateName string `long:"template" short:"t" description:"path to template"` + GithubToken string `long:"github-token" short:"g" description:"auth token for fetching issues merged between releases" env:"GITHUB_TOKEN"` + GithubHost string `long:"github-host" description:"set this when you are using GitHub enterprise" 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"` notes.IssuesQuery notes.TrainstatQuery } @@ -43,19 +47,21 @@ type ReleaseNotes struct { stat func(name string) (fs.FileInfo, error) io.Writer - fetchNotesData FetchNotesData + fetchNotesData FetchNotesData + variablesService baking.TemplateVariablesService 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) (notes.Data, error) +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) func NewReleaseNotesCommand() (ReleaseNotes, error) { return ReleaseNotes{ - fetchNotesData: notes.FetchData, - readFile: os.ReadFile, - Writer: os.Stdout, - stat: os.Stat, + variablesService: baking.NewTemplateVariablesService(osfs.New(".")), + fetchNotesData: notes.FetchData, + readFile: os.ReadFile, + Writer: os.Stdout, + stat: os.Stat, }, nil } @@ -73,6 +79,16 @@ func (r ReleaseNotes) Execute(args []string) error { return err } + templateVariables, err := r.variablesService.FromPathsAndPairs(r.Options.VariableFiles, r.Options.Variables) + if err != nil { + return fmt.Errorf("failed to parse template variables: %s", err) + } + if varValue, ok := templateVariables["github_token"]; !ok && r.Options.GithubToken != "" { + templateVariables["github_token"] = r.Options.GithubToken + } else if ok && r.Options.GithubToken == "" { + r.Options.GithubToken = varValue.(string) + } + ctx := context.Background() if err := r.initRepo(); err != nil { @@ -101,6 +117,7 @@ func (r ReleaseNotes) Execute(args []string) error { nonFlagArgs[0], nonFlagArgs[1], r.Options.IssuesQuery, &trainstatClient, + templateVariables, ) if err != nil { return err diff --git a/internal/commands/release_notes_test.go b/internal/commands/release_notes_test.go index 04059357b..8af9f6bbf 100644 --- a/internal/commands/release_notes_test.go +++ b/internal/commands/release_notes_test.go @@ -72,7 +72,7 @@ func TestReleaseNotes_Execute(t *testing.T) { 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) (notes.Data, error) { + 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) { ctx, repository, client = c, repo, ghc tileRepoOwner, tileRepoName, kilnfilePath, initialRevision, finalRevision = tro, trn, kfp, ir, fr issuesQuery = iq @@ -94,7 +94,7 @@ func TestReleaseNotes_Execute(t *testing.T) { {BOSHReleaseTarballLock: cargo.BOSHReleaseTarballLock{Name: "lemon", Version: "1.1.0"}}, }, Bumps: cargo.BumpList{ - {Name: "banana", FromVersion: "1.1.0", ToVersion: "1.2.0"}, + {Name: "banana", From: cargo.BOSHReleaseTarballLock{Version: "1.1.0"}, To: cargo.BOSHReleaseTarballLock{Version: "1.2.0"}}, }, TrainstatNotes: []string{ "* **[Feature]** this is a feature.", diff --git a/internal/component/github_release_source.go b/internal/component/github_release_source.go index 204d701a3..0c7ab317f 100644 --- a/internal/component/github_release_source.go +++ b/internal/component/github_release_source.go @@ -16,7 +16,6 @@ import ( "github.com/Masterminds/semver/v3" "github.com/google/go-github/v50/github" - "golang.org/x/oauth2" "github.com/pivotal-cf/kiln/internal/gh" "github.com/pivotal-cf/kiln/pkg/cargo" @@ -38,26 +37,16 @@ func NewGithubReleaseSource(c cargo.ReleaseSourceConfig) *GithubReleaseSource { if c.Type != "" && c.Type != ReleaseSourceTypeGithub { panic(panicMessageWrongReleaseSourceType) } - if c.GithubToken == "" { + if c.GithubToken == "" { // TODO remove this panic("no token passed for github release source") } if c.Org == "" { panic("no github org passed for github release source") } - ctx := context.TODO() - tokenSource := oauth2.StaticTokenSource(&oauth2.Token{AccessToken: c.GithubToken}) - tokenClient := oauth2.NewClient(ctx, tokenSource) - var githubClient *github.Client - if c.Endpoint != "" { - var err error - githubClient, err = github.NewEnterpriseClient(c.Endpoint, c.Endpoint, tokenClient) - if err != nil { - panic(err) - } - } else { - githubClient = github.NewClient(tokenClient) + githubClient, err := c.GitHubClient(context.TODO()) + if err != nil { + panic(err) } - return &GithubReleaseSource{ ReleaseSourceConfig: c, Token: c.GithubToken, diff --git a/pkg/cargo/bump.go b/pkg/cargo/bump.go index 0daf74558..16af59e33 100644 --- a/pkg/cargo/bump.go +++ b/pkg/cargo/bump.go @@ -2,6 +2,7 @@ package cargo import ( "context" + "fmt" "slices" "sort" "strings" @@ -14,7 +15,8 @@ import ( ) type Bump struct { - Name, FromVersion, ToVersion string + Name string + From, To BOSHReleaseTarballLock Releases []*github.RepositoryRelease } @@ -34,6 +36,9 @@ func (bump Bump) ReleaseNotes() string { return strings.TrimSpace(s.String()) } +func (bump Bump) ToVersion() string { return bump.To.Version } +func (bump Bump) FromVersion() string { return bump.From.Version } + func deduplicateReleasesWithTheSameTagName(bump Bump) Bump { updated := bump updated.Releases = slices.Clone(bump.Releases) @@ -67,9 +72,9 @@ func CalculateBumps(current, previous []BOSHReleaseTarballLock) []Bump { continue } bumps = append(bumps, Bump{ - Name: c.Name, - FromVersion: p.Version, - ToVersion: c.Version, + Name: c.Name, + From: p, + To: c, }) } return bumps @@ -78,8 +83,8 @@ func CalculateBumps(current, previous []BOSHReleaseTarballLock) []Bump { func WinfsVersionBump(bumped bool, version string, bumps []Bump) []Bump { if bumped { bumps = append(bumps, Bump{ - Name: "windowsfs-release", - ToVersion: version, + Name: "windowsfs-release", + To: BOSHReleaseTarballLock{Version: version}, }) } return bumps @@ -87,11 +92,11 @@ func WinfsVersionBump(bumped bool, version string, bumps []Bump) []Bump { func (bump Bump) toFrom() (to, from *semver.Version, _ error) { var err error - from, err = semver.NewVersion(bump.FromVersion) + from, err = semver.NewVersion(bump.From.Version) if err != nil { return nil, nil, err } - to, err = semver.NewVersion(bump.ToVersion) + to, err = semver.NewVersion(bump.To.Version) if err != nil { return nil, nil, err } @@ -107,9 +112,9 @@ func (list BumpList) ForLock(lock BOSHReleaseTarballLock) Bump { } } return Bump{ - Name: lock.Name, - FromVersion: lock.Version, - ToVersion: lock.Version, + Name: lock.Name, + From: lock, + To: lock, } } @@ -121,9 +126,12 @@ type repositoryReleaseLister interface { ListReleases(ctx context.Context, owner, repo string, opts *github.ListOptions) ([]*github.RepositoryRelease, *github.Response, error) } -type RepositoryReleaseLister = repositoryReleaseLister +type ( + RepositoryReleaseLister = repositoryReleaseLister + githubClientFunc func(Kilnfile, BOSHReleaseTarballLock) (repositoryReleaseLister, error) +) -func ReleaseNotes(ctx context.Context, repoService RepositoryReleaseLister, kf Kilnfile, list BumpList) (BumpList, error) { +func releaseNotes(ctx context.Context, kf Kilnfile, list BumpList, client githubClientFunc) (BumpList, error) { const workerCount = 10 type fetchReleaseNotesForBump struct { @@ -143,7 +151,7 @@ func ReleaseNotes(ctx context.Context, repoService RepositoryReleaseLister, kf K go func() { defer wg.Done() for j := range in { - j.bump = fetchReleasesForBump(ctx, repoService, kf, j.bump) + j.bump = fetchReleasesForBump(ctx, kf, j.bump, client) results <- j } }() @@ -174,6 +182,23 @@ func ReleaseNotes(ctx context.Context, repoService RepositoryReleaseLister, kf K return list, nil } +func ReleaseNotes(ctx context.Context, kf Kilnfile, list BumpList) (BumpList, error) { + return releaseNotes(ctx, kf, list, func(kilnfile Kilnfile, lock BOSHReleaseTarballLock) (repositoryReleaseLister, error) { + i := slices.IndexFunc(kf.ReleaseSources, func(config ReleaseSourceConfig) bool { + return BOSHReleaseTarballSourceID(config) == lock.RemoteSource + }) + if i < 0 { + return nil, fmt.Errorf("release source with id %s not found", lock.RemoteSource) + } + source := kf.ReleaseSources[i] + client, err := source.GitHubClient(ctx) + if err != nil { + return nil, err + } + return client.Repositories, err + }) +} + func fetchReleasesFromRepo(ctx context.Context, repoService RepositoryReleaseLister, repository string, from, to *semver.Version) []*github.RepositoryRelease { owner, repo, err := gh.RepositoryOwnerAndNameFromPath(repository) if err != nil { @@ -196,7 +221,11 @@ func fetchReleasesFromRepo(ctx context.Context, repoService RepositoryReleaseLis return result } -func fetchReleasesForBump(ctx context.Context, repoService RepositoryReleaseLister, kf Kilnfile, bump Bump) Bump { +func fetchReleasesForBump(ctx context.Context, kf Kilnfile, bump Bump, client githubClientFunc) Bump { + lister, err := client(kf, bump.To) + if err != nil { + return bump + } spec, err := kf.BOSHReleaseTarballSpecification(bump.Name) if err != nil { return bump @@ -208,7 +237,7 @@ func fetchReleasesForBump(ctx context.Context, repoService RepositoryReleaseList } if spec.GitHubRepository != "" { - releases := fetchReleasesFromRepo(ctx, repoService, spec.GitHubRepository, from, to) + releases := fetchReleasesFromRepo(ctx, lister, spec.GitHubRepository, from, to) bump.Releases = append(bump.Releases, releases...) } diff --git a/pkg/cargo/bump_test.go b/pkg/cargo/bump_test.go index 6b24d627f..edafaba80 100644 --- a/pkg/cargo/bump_test.go +++ b/pkg/cargo/bump_test.go @@ -33,7 +33,7 @@ func TestCalculateBumps(t *testing.T) { {Name: "a", Version: "1"}, {Name: "b", Version: "1"}, })).To(Equal([]Bump{ - {Name: "b", FromVersion: "1", ToVersion: "2"}, + {Name: "b", From: BOSHReleaseTarballLock{Name: "b", Version: "1"}, To: BOSHReleaseTarballLock{Name: "b", Version: "2"}}, }), "it returns the changed lock", ) @@ -49,8 +49,8 @@ func TestCalculateBumps(t *testing.T) { {Name: "b", Version: "1"}, {Name: "c", Version: "1"}, })).To(Equal([]Bump{ - {Name: "a", FromVersion: "1", ToVersion: "2"}, - {Name: "c", FromVersion: "1", ToVersion: "2"}, + {Name: "a", From: BOSHReleaseTarballLock{Name: "a", Version: "1"}, To: BOSHReleaseTarballLock{Name: "a", Version: "2"}}, + {Name: "c", From: BOSHReleaseTarballLock{Name: "c", Version: "1"}, To: BOSHReleaseTarballLock{Name: "c", Version: "2"}}, }), "it returns all the bumps", ) @@ -77,7 +77,7 @@ func TestCalculateBumps(t *testing.T) { }, []BOSHReleaseTarballLock{ {Name: "a", Version: "1"}, })).To(Equal([]Bump{ - {Name: "b", FromVersion: "", ToVersion: "1"}, + {Name: "b", From: BOSHReleaseTarballLock{}, To: BOSHReleaseTarballLock{Name: "b", Version: "1"}}, }), "it returns the component as a bump", ) @@ -90,18 +90,18 @@ func TestWinfsVersionBump(t *testing.T) { t.Run("when the winfs version is not bumped", func(t *testing.T) { please.Expect(WinfsVersionBump(false, "2.61.0", []Bump{ - {Name: "b", FromVersion: "1", ToVersion: "2"}, + {Name: "b", From: BOSHReleaseTarballLock{Version: "1"}, To: BOSHReleaseTarballLock{Version: "2"}}, })).To(Equal([]Bump{ - {Name: "b", FromVersion: "1", ToVersion: "2"}, + {Name: "b", From: BOSHReleaseTarballLock{Version: "1"}, To: BOSHReleaseTarballLock{Version: "2"}}, })) }) t.Run("when the winfs version is bumped", func(t *testing.T) { please.Expect(WinfsVersionBump(true, "2.61.0", []Bump{ - {Name: "b", FromVersion: "1", ToVersion: "2"}, + {Name: "b", From: BOSHReleaseTarballLock{Version: "1"}, To: BOSHReleaseTarballLock{Version: "2"}}, })).To(Equal([]Bump{ - {Name: "b", FromVersion: "1", ToVersion: "2"}, - {Name: "windowsfs-release", FromVersion: "", ToVersion: "2.61.0"}, + {Name: "b", From: BOSHReleaseTarballLock{Version: "1"}, To: BOSHReleaseTarballLock{Version: "2"}}, + {Name: "windowsfs-release", From: BOSHReleaseTarballLock{Version: ""}, To: BOSHReleaseTarballLock{Version: "2.61.0"}}, })) }) } @@ -140,9 +140,8 @@ func TestInternal_addReleaseNotes(t *testing.T) { return nil, nil, nil } - result, err := ReleaseNotes( + result, err := releaseNotes( context.Background(), - releaseLister, Kilnfile{ Releases: []BOSHReleaseTarballSpecification{ { @@ -156,15 +155,17 @@ func TestInternal_addReleaseNotes(t *testing.T) { }, BumpList{ { - Name: "peach", - ToVersion: "2.0.1", // served - FromVersion: "0.1.3", // ripe + Name: "peach", + To: BOSHReleaseTarballLock{Version: "2.0.1"}, // served + From: BOSHReleaseTarballLock{Version: "0.1.3"}, // ripe }, { - Name: "mango", - ToVersion: "10", - FromVersion: "9", + Name: "mango", + To: BOSHReleaseTarballLock{Version: "10"}, + From: BOSHReleaseTarballLock{Version: "9"}, }, + }, func(kilnfile Kilnfile, lock BOSHReleaseTarballLock) (repositoryReleaseLister, error) { + return releaseLister, nil }) please.Expect(err).NotTo(HaveOccurred()) please.Expect(result).To(HaveLen(2)) @@ -174,6 +175,37 @@ func TestInternal_addReleaseNotes(t *testing.T) { please.Expect(result[0].ReleaseNotes()).To(Equal("served\nplated\nstored\nlabeled\npreserved\nchopped\ncleaned")) } +func Test_deduplicateReleasesWithTheSameTagName(t *testing.T) { + please := NewWithT(t) + b := Bump{ + Releases: []*github.RepositoryRelease{ + {TagName: ptr("Y")}, + {TagName: ptr("1")}, + {TagName: ptr("2")}, + {TagName: ptr("3")}, + {TagName: ptr("3")}, + {TagName: ptr("3")}, + {TagName: ptr("X")}, + {TagName: ptr("2")}, + {TagName: ptr("4")}, + {TagName: ptr("4")}, + }, + } + b = deduplicateReleasesWithTheSameTagName(b) + tags := make([]string, 0, len(b.Releases)) + for _, r := range b.Releases { + tags = append(tags, r.GetTagName()) + } + please.Expect(tags).To(Equal([]string{ + "Y", + "1", + "2", + "3", + "X", + "4", + })) +} + func githubResponse(t *testing.T, status int) *github.Response { t.Helper() diff --git a/pkg/cargo/files.go b/pkg/cargo/files.go index 80f753930..7deecc01f 100644 --- a/pkg/cargo/files.go +++ b/pkg/cargo/files.go @@ -20,6 +20,8 @@ func InterpolateAndParseKilnfile(in io.Reader, templateVariables map[string]any) return Kilnfile{}, fmt.Errorf("unable to read Kilnfile: %w", err) } + fmt.Println(string(kilnfileYAML)) + kilnfileTemplate, err := template.New("Kilnfile"). Funcs(template.FuncMap{ "variable": variableTemplateFunction(templateVariables), @@ -28,7 +30,7 @@ func InterpolateAndParseKilnfile(in io.Reader, templateVariables map[string]any) Option("missingkey=error"). Parse(string(kilnfileYAML)) if err != nil { - return Kilnfile{}, err + return Kilnfile{}, fmt.Errorf("failed to interpolate kilnfile using text/template: %w", err) } var buf bytes.Buffer diff --git a/pkg/cargo/kilnfile.go b/pkg/cargo/kilnfile.go index 4622543e2..dc9212a72 100644 --- a/pkg/cargo/kilnfile.go +++ b/pkg/cargo/kilnfile.go @@ -1,8 +1,11 @@ package cargo import ( + "context" "errors" "fmt" + "github.com/google/go-github/v50/github" + "golang.org/x/oauth2" "strings" "gopkg.in/yaml.v3" @@ -169,6 +172,20 @@ type ReleaseSourceConfig struct { Password string `yaml:"password,omitempty"` } +func (c ReleaseSourceConfig) GitHubClient(ctx context.Context) (*github.Client, error) { + if c.GithubToken == "" { + return nil, errors.New("no token passed for github release source") + } + tokenSource := oauth2.StaticTokenSource(&oauth2.Token{AccessToken: c.GithubToken}) + tokenClient := oauth2.NewClient(ctx, tokenSource) + var githubClient *github.Client + if c.Endpoint != "" { + return github.NewEnterpriseClient(c.Endpoint, c.Endpoint, tokenClient) + } + githubClient = github.NewClient(tokenClient) + return githubClient, nil +} + // BOSHReleaseTarballLock represents an exact build of a bosh release // It may identify the where the release is cached; // it may identify the stemcell used to compile the release. diff --git a/pkg/notes/notes_data.go b/pkg/notes/notes_data.go index 23830ed07..b44b5b868 100644 --- a/pkg/notes/notes_data.go +++ b/pkg/notes/notes_data.go @@ -127,8 +127,8 @@ func (q IssuesQuery) Exp() (*regexp.Regexp, error) { return regexp.Compile(str) } -func FetchData(ctx context.Context, repo *git.Repository, client *github.Client, tileRepoOwner, tileRepoName, kilnfilePath, initialRevision, finalRevision string, issuesQuery IssuesQuery, trainstatClient TrainstatNotesFetcher) (Data, error) { - f, err := newFetchNotesData(repo, tileRepoOwner, tileRepoName, kilnfilePath, initialRevision, finalRevision, client, issuesQuery, trainstatClient) +func FetchData(ctx context.Context, repo *git.Repository, client *github.Client, tileRepoOwner, tileRepoName, kilnfilePath, initialRevision, finalRevision string, issuesQuery IssuesQuery, trainstatClient TrainstatNotesFetcher, variables map[string]any) (Data, error) { + f, err := newFetchNotesData(repo, tileRepoOwner, tileRepoName, kilnfilePath, initialRevision, finalRevision, client, issuesQuery, trainstatClient, variables) if err != nil { return Data{}, err } @@ -138,11 +138,10 @@ func FetchData(ctx context.Context, repo *git.Repository, client *github.Client, // FetchDataWithoutRepo can be used to generate release notes from tile metadata func FetchDataWithoutRepo(ctx context.Context, client *github.Client, tileRepoOwner, tileRepoName string, kilnfile cargo.Kilnfile, kilnfileLockInitial, kilnfileLockFinal cargo.KilnfileLock, issuesQuery IssuesQuery) (Data, error) { r := fetchNotesData{ - repoOwner: tileRepoOwner, - repoName: tileRepoName, - issuesQuery: issuesQuery, - issuesService: client.Issues, - releasesService: client.Repositories, + repoOwner: tileRepoOwner, + repoName: tileRepoName, + issuesQuery: issuesQuery, + issuesService: client.Issues, } data := Data{ Bumps: cargo.CalculateBumps(kilnfileLockFinal.Releases, kilnfileLockInitial.Releases), @@ -164,7 +163,7 @@ func FetchDataWithoutRepo(ctx context.Context, client *github.Client, tileRepoOw return data, nil } -func newFetchNotesData(repo *git.Repository, tileRepoOwner string, tileRepoName string, kilnfilePath string, initialRevision string, finalRevision string, client *github.Client, issuesQuery IssuesQuery, trainstatClient TrainstatNotesFetcher) (fetchNotesData, error) { +func newFetchNotesData(repo *git.Repository, tileRepoOwner string, tileRepoName string, kilnfilePath string, initialRevision string, finalRevision string, client *github.Client, issuesQuery IssuesQuery, trainstatClient TrainstatNotesFetcher, variables map[string]any) (fetchNotesData, error) { if repo == nil { return fetchNotesData{}, errors.New("git repository required to generate release notes") } @@ -184,11 +183,12 @@ func newFetchNotesData(repo *git.Repository, tileRepoOwner string, tileRepoName issuesQuery: issuesQuery, trainstatClient: trainstatClient, + + variables: variables, } if client != nil { f.issuesService = client.Issues - f.releasesService = client.Repositories } return f, nil } @@ -201,7 +201,6 @@ type fetchNotesData struct { repository *git.Repository issuesService - releasesService cargo.RepositoryReleaseLister repoOwner, repoName, kilnfilePath, @@ -209,6 +208,8 @@ type fetchNotesData struct { issuesQuery IssuesQuery trainstatClient TrainstatNotesFetcher + + variables map[string]any } func (r fetchNotesData) fetch(ctx context.Context) (Data, error) { @@ -359,10 +360,20 @@ type issuesService interface { // test for Execute does not set GithubToken intentionally so this code is not triggered and Execute does not actually // reach out to GitHub. func (r fetchNotesData) fetchIssuesAndReleaseNotes(ctx context.Context, finalKF, wtKF cargo.Kilnfile, bumpList cargo.BumpList, issuesQuery IssuesQuery) ([]*github.Issue, cargo.BumpList, error) { - if r.releasesService == nil || r.issuesService == nil { + if r.issuesService == nil { return nil, bumpList, nil } - bumpList, err := cargo.ReleaseNotes(ctx, r.releasesService, setEmptyComponentGitHubRepositoryFromOtherKilnfile(finalKF, wtKF), bumpList) + + buf, err := yaml.Marshal(finalKF) + if err != nil { + return nil, nil, err + } + finalKF, err = cargo.InterpolateAndParseKilnfile(bytes.NewReader(buf), r.variables) + if err != nil { + return nil, nil, err + } + + bumpList, err = cargo.ReleaseNotes(ctx, setEmptyComponentGitHubRepositoryFromOtherKilnfile(finalKF, wtKF), bumpList) if err != nil { return nil, nil, err } diff --git a/pkg/notes/notes_data_test.go b/pkg/notes/notes_data_test.go index a5372bfa1..6a2c51416 100644 --- a/pkg/notes/notes_data_test.go +++ b/pkg/notes/notes_data_test.go @@ -115,7 +115,6 @@ func Test_fetch_for_ist_tas(t *testing.T) { historicVersion: historicVersion.Spy, issuesService: fakeIssuesService, - releasesService: fakeReleaseService, trainstatClient: fakeTrainstatClient, } @@ -129,7 +128,6 @@ func Test_fetch_for_ist_tas(t *testing.T) { please.Expect(historicVersion.CallCount()).To(Equal(1)) _, historicVersionHashArg, _ := historicVersion.ArgsForCall(0) please.Expect(historicVersionHashArg).To(Equal(finalHash)) - please.Expect(fakeReleaseService.ListReleasesCallCount()).To(Equal(1)) please.Expect(fakeIssuesService.GetCallCount()).To(Equal(2)) _, orgName, repoName, n := fakeIssuesService.GetArgsForCall(0) @@ -245,7 +243,6 @@ func Test_fetch_for_tasw(t *testing.T) { historicVersion: historicVersion.Spy, issuesService: fakeIssuesService, - releasesService: fakeReleaseService, trainstatClient: fakeTrainstatClient, } @@ -259,7 +256,6 @@ func Test_fetch_for_tasw(t *testing.T) { please.Expect(historicVersion.CallCount()).To(Equal(1)) _, historicVersionHashArg, _ := historicVersion.ArgsForCall(0) please.Expect(historicVersionHashArg).To(Equal(finalHash)) - please.Expect(fakeReleaseService.ListReleasesCallCount()).To(Equal(1)) please.Expect(fakeIssuesService.GetCallCount()).To(Equal(2)) _, orgName, repoName, n := fakeIssuesService.GetArgsForCall(0) diff --git a/pkg/notes/notes_page_test.go b/pkg/notes/notes_page_test.go index 5488881d7..c0ebad3f4 100644 --- a/pkg/notes/notes_page_test.go +++ b/pkg/notes/notes_page_test.go @@ -121,22 +121,22 @@ func TestParseNotesPage(t *testing.T) { {BOSHReleaseTarballLock: cargo.BOSHReleaseTarballLock{Name: "uaa", Version: "73.4.32"}}, }, Bumps: cargo.BumpList{ - {Name: "backup-and-restore-sdk", ToVersion: "1.18.26"}, - {Name: "bpm", ToVersion: "1.1.15"}, - {Name: "capi", ToVersion: "1.84.20"}, - {Name: "cf-autoscaling", ToVersion: "241"}, - {Name: "cf-networking", ToVersion: "2.40.0"}, - {Name: "cflinuxfs3", ToVersion: "0.264.0"}, - {Name: "dotnet-core-offline-buildpack", ToVersion: "2.3.36"}, - {Name: "go-offline-buildpack", ToVersion: "1.9.37"}, - {Name: "nodejs-offline-buildpack", ToVersion: "1.7.63"}, - {Name: "php-offline-buildpack", ToVersion: "4.4.48"}, - {Name: "python-offline-buildpack", ToVersion: "1.7.47"}, - {Name: "r-offline-buildpack", ToVersion: "1.1.23"}, - {Name: "routing", ToVersion: "0.226.0"}, - {Name: "ruby-offline-buildpack", ToVersion: "1.8.48"}, - {Name: "silk", ToVersion: "2.40.0"}, - {Name: "staticfile-offline-buildpack", ToVersion: "1.5.26"}, + {Name: "backup-and-restore-sdk", To: cargo.BOSHReleaseTarballLock{Name: "backup-and-restore-sdk", Version: "1.18.26"}}, + {Name: "bpm", To: cargo.BOSHReleaseTarballLock{Name: "bpm", Version: "1.1.15"}}, + {Name: "capi", To: cargo.BOSHReleaseTarballLock{Name: "capi", Version: "1.84.20"}}, + {Name: "cf-autoscaling", To: cargo.BOSHReleaseTarballLock{Name: "cf-autoscaling", Version: "241"}}, + {Name: "cf-networking", To: cargo.BOSHReleaseTarballLock{Name: "cf-networking", Version: "2.40.0"}}, + {Name: "cflinuxfs3", To: cargo.BOSHReleaseTarballLock{Name: "cflinuxfs3", Version: "0.264.0"}}, + {Name: "dotnet-core-offline-buildpack", To: cargo.BOSHReleaseTarballLock{Name: "dotnet-core-offline-buildpack", Version: "2.3.36"}}, + {Name: "go-offline-buildpack", To: cargo.BOSHReleaseTarballLock{Name: "go-offline-buildpack", Version: "1.9.37"}}, + {Name: "nodejs-offline-buildpack", To: cargo.BOSHReleaseTarballLock{Name: "nodejs-offline-buildpack", Version: "1.7.63"}}, + {Name: "php-offline-buildpack", To: cargo.BOSHReleaseTarballLock{Name: "php-offline-buildpack", Version: "4.4.48"}}, + {Name: "python-offline-buildpack", To: cargo.BOSHReleaseTarballLock{Name: "python-offline-buildpack", Version: "1.7.47"}}, + {Name: "r-offline-buildpack", To: cargo.BOSHReleaseTarballLock{Name: "r-offline-buildpack", Version: "1.1.23"}}, + {Name: "routing", To: cargo.BOSHReleaseTarballLock{Name: "routing", Version: "0.226.0"}}, + {Name: "ruby-offline-buildpack", To: cargo.BOSHReleaseTarballLock{Name: "ruby-offline-buildpack", Version: "1.8.48"}}, + {Name: "silk", To: cargo.BOSHReleaseTarballLock{Name: "silk", Version: "2.40.0"}}, + {Name: "staticfile-offline-buildpack", To: cargo.BOSHReleaseTarballLock{Name: "staticfile-offline-buildpack", Version: "1.5.26"}}, }, } @@ -208,7 +208,7 @@ func Test_newFetchNotesData(t *testing.T) { IssueMilestone: "BLA", }, &TrainstatClient{ host: "test", - }) + }, make(map[string]any)) please.Expect(err).NotTo(HaveOccurred()) please.Expect(f.repoOwner).To(Equal("o")) please.Expect(f.repoName).To(Equal("r")) @@ -226,14 +226,14 @@ func Test_newFetchNotesData(t *testing.T) { }) t.Run("when repo is nil", func(t *testing.T) { please := NewWithT(t) - _, err := newFetchNotesData(nil, "o", "r", "k", "ri", "rf", &github.Client{}, IssuesQuery{}, &TrainstatClient{}) + _, err := newFetchNotesData(nil, "o", "r", "k", "ri", "rf", &github.Client{}, IssuesQuery{}, &TrainstatClient{}, make(map[string]any)) please.Expect(err).To(HaveOccurred()) }) t.Run("when repo is not nil", func(t *testing.T) { please := NewWithT(t) f, err := newFetchNotesData(&git.Repository{ Storer: &memory.Storage{}, - }, "o", "r", "k", "ri", "rf", &github.Client{}, IssuesQuery{}, &TrainstatClient{}) + }, "o", "r", "k", "ri", "rf", &github.Client{}, IssuesQuery{}, &TrainstatClient{}, make(map[string]any)) please.Expect(err).NotTo(HaveOccurred()) please.Expect(f.repository).NotTo(BeNil()) please.Expect(f.revisionResolver).NotTo(BeNil()) @@ -244,17 +244,15 @@ func Test_newFetchNotesData(t *testing.T) { f, err := newFetchNotesData(&git.Repository{}, "o", "r", "k", "ri", "rf", &github.Client{ Issues: &github.IssuesService{}, Repositories: &github.RepositoriesService{}, - }, IssuesQuery{}, &TrainstatClient{}) + }, IssuesQuery{}, &TrainstatClient{}, make(map[string]any)) please.Expect(err).NotTo(HaveOccurred()) please.Expect(f.issuesService).NotTo(BeNil()) - please.Expect(f.releasesService).NotTo(BeNil()) }) t.Run("when github client is nil", func(t *testing.T) { please := NewWithT(t) - f, err := newFetchNotesData(&git.Repository{}, "o", "r", "k", "ri", "rf", nil, IssuesQuery{}, &TrainstatClient{}) + f, err := newFetchNotesData(&git.Repository{}, "o", "r", "k", "ri", "rf", nil, IssuesQuery{}, &TrainstatClient{}, make(map[string]any)) please.Expect(err).NotTo(HaveOccurred()) please.Expect(f.issuesService).To(BeNil()) - please.Expect(f.releasesService).To(BeNil()) }) }