Skip to content

Commit 39a43eb

Browse files
davewalterrizwanreza
davewalter
authored and
rizwanreza
committed
Standardise release source logging
All release sources now define their own logger by default, but allow tests to pass in a custom logger for cleaner output.
1 parent 1d7b2de commit 39a43eb

11 files changed

+87
-67
lines changed

‎internal/component/artifactory.go renamed to ‎internal/component/artifactory_release_source.go

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020
"text/template"
2121

2222
"github.com/Masterminds/semver/v3"
23+
2324
"github.com/pivotal-cf/kiln/pkg/cargo"
2425
)
2526

@@ -53,18 +54,22 @@ type ArtifactoryFileInfo struct {
5354

5455
// NewArtifactoryReleaseSource will provision a new ArtifactoryReleaseSource Project
5556
// from the Kilnfile (ReleaseSourceConfig). If type is incorrect it will PANIC
56-
func NewArtifactoryReleaseSource(c cargo.ReleaseSourceConfig) *ArtifactoryReleaseSource {
57+
func NewArtifactoryReleaseSource(c cargo.ReleaseSourceConfig, logger *log.Logger) *ArtifactoryReleaseSource {
5758
if c.Type != "" && c.Type != ReleaseSourceTypeArtifactory {
5859
panic(panicMessageWrongReleaseSourceType)
5960
}
6061

6162
// ctx := context.TODO()
6263

64+
if logger == nil {
65+
logger = log.New(os.Stderr, "[Artifactory release source] ", log.Default().Flags())
66+
}
67+
6368
return &ArtifactoryReleaseSource{
6469
Client: http.DefaultClient,
6570
ReleaseSourceConfig: c,
6671
ID: c.ID,
67-
logger: log.New(os.Stderr, "[Artifactory release source] ", log.Default().Flags()),
72+
logger: logger,
6873
}
6974
}
7075

‎internal/component/artifactory_internal_test.go renamed to ‎internal/component/artifactory_release_source_internal_test.go

File renamed without changes.

‎internal/component/artifactory_test.go renamed to ‎internal/component/artifactory_release_source_test.go

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -54,9 +54,10 @@ var _ = Describe("interacting with BOSH releases on Artifactory", func() {
5454
})
5555
})
5656
JustBeforeEach(func() {
57+
logger := log.New(GinkgoWriter, "", 0)
5758
server = httptest.NewServer(artifactoryRouter)
5859
config.ArtifactoryHost = server.URL
59-
source = component.NewArtifactoryReleaseSource(config)
60+
source = component.NewArtifactoryReleaseSource(config, logger)
6061
source.Client = server.Client()
6162
})
6263
AfterEach(func() {
@@ -142,8 +143,9 @@ var _ = Describe("interacting with BOSH releases on Artifactory", func() {
142143
})
143144
When("the server URL ends in /artifactory", func() {
144145
JustBeforeEach(func() {
146+
logger := log.New(GinkgoWriter, "", 0)
145147
config.ArtifactoryHost = server.URL + "/artifactory"
146-
source = component.NewArtifactoryReleaseSource(config)
148+
source = component.NewArtifactoryReleaseSource(config, logger)
147149
source.Client = server.Client()
148150
})
149151

@@ -162,8 +164,9 @@ var _ = Describe("interacting with BOSH releases on Artifactory", func() {
162164
})
163165
When("the server URL is malformed", func() {
164166
JustBeforeEach(func() {
167+
logger := log.New(GinkgoWriter, "", 0)
165168
config.ArtifactoryHost = ":improper-url/formatting"
166-
source = component.NewArtifactoryReleaseSource(config)
169+
source = component.NewArtifactoryReleaseSource(config, logger)
167170
source.Client = server.Client()
168171
})
169172
It("returns an error", func() {

‎internal/component/bosh_io_release_source.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212
"path/filepath"
1313

1414
"github.com/Masterminds/semver/v3"
15+
1516
"github.com/pivotal-cf/kiln/pkg/cargo"
1617
)
1718

@@ -61,9 +62,11 @@ func NewBOSHIOReleaseSource(c cargo.ReleaseSourceConfig, customServerURI string,
6162
if customServerURI == "" {
6263
customServerURI = "https://bosh.io"
6364
}
65+
6466
if logger == nil {
6567
logger = log.New(os.Stderr, "[bosh.io release source] ", log.Default().Flags())
6668
}
69+
6770
return &BOSHIOReleaseSource{
6871
ReleaseSourceConfig: c,
6972
logger: logger,

‎internal/component/github_release_source.go

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,13 +33,15 @@ type GithubReleaseSource struct {
3333

3434
// NewGithubReleaseSource will provision a new GithubReleaseSource Project
3535
// from the Kilnfile (ReleaseSourceConfig). If type is incorrect it will PANIC
36-
func NewGithubReleaseSource(c cargo.ReleaseSourceConfig) *GithubReleaseSource {
36+
func NewGithubReleaseSource(c cargo.ReleaseSourceConfig, logger *log.Logger) *GithubReleaseSource {
3737
if c.Type != "" && c.Type != ReleaseSourceTypeGithub {
3838
panic(panicMessageWrongReleaseSourceType)
3939
}
40+
4041
if c.GithubToken == "" { // TODO remove this
4142
panic("no token passed for github release source")
4243
}
44+
4345
if c.Org == "" {
4446
panic("no github org passed for github release source")
4547
}
@@ -53,14 +55,18 @@ func NewGithubReleaseSource(c cargo.ReleaseSourceConfig) *GithubReleaseSource {
5355
host = "https://github.gwd.broadcom.net"
5456
}
5557

58+
if logger == nil {
59+
logger = log.New(os.Stderr, "[Github release source] ", log.Default().Flags())
60+
}
61+
5662
githubClient, err := gh.GitClient(context.TODO(), host, c.GithubToken, c.GithubToken)
5763
if err != nil {
5864
panic(err)
5965
}
6066
return &GithubReleaseSource{
6167
ReleaseSourceConfig: c,
6268
Token: c.GithubToken,
63-
Logger: log.New(os.Stderr, "[Github release source] ", log.Default().Flags()),
69+
Logger: logger,
6470

6571
ReleaseAssetDownloader: githubClient.Repositories,
6672
ReleaseByTagGetter: githubClient.Repositories,

‎internal/component/github_release_source_test.go

Lines changed: 37 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212
"testing"
1313

1414
"github.com/google/go-github/v50/github"
15+
. "github.com/onsi/ginkgo"
1516
. "github.com/onsi/gomega"
1617

1718
"github.com/pivotal-cf/kiln/internal/component"
@@ -22,11 +23,16 @@ import (
2223
func TestListAllOfTheCrap(t *testing.T) {
2324
t.SkipNow()
2425

25-
grs := component.NewGithubReleaseSource(cargo.ReleaseSourceConfig{
26-
Type: component.ReleaseSourceTypeGithub,
27-
GithubToken: os.Getenv("GITHUB_ACCESS_TOKEN"),
28-
Org: "cloudfoundry",
29-
})
26+
logger := log.New(GinkgoWriter, "[test] ", log.Default().Flags())
27+
28+
grs := component.NewGithubReleaseSource(
29+
cargo.ReleaseSourceConfig{
30+
Type: component.ReleaseSourceTypeGithub,
31+
GithubToken: os.Getenv("GITHUB_ACCESS_TOKEN"),
32+
Org: "cloudfoundry",
33+
},
34+
logger,
35+
)
3036
// grs.ListAllOfTheCrap(context.TODO(), "cloudfoundry")
3137

3238
// grs.Client.Repositories.GetReleaseByTag()
@@ -88,9 +94,8 @@ func TestGithubReleaseSource_ComponentLockFromGithubRelease(t *testing.T) {
8894
file := &SetTrueOnClose{Reader: bytes.NewBufferString("hello")}
8995
downloader.DownloadReleaseAssetReturns(file, "", nil)
9096

91-
output := bytes.NewBuffer(nil)
9297
grsMock := &component.GithubReleaseSource{
93-
Logger: log.New(output, "[Github release source] ", log.Default().Flags()),
98+
Logger: log.New(GinkgoWriter, "[Github release source] ", log.Default().Flags()),
9499
ReleaseAssetDownloader: downloader,
95100
ReleaseByTagGetter: releaseGetter,
96101
ReleaseSourceConfig: cargo.ReleaseSourceConfig{
@@ -177,9 +182,8 @@ func TestGithubReleaseSource_ComponentLockFromGithubRelease(t *testing.T) {
177182
file := &SetTrueOnClose{Reader: bytes.NewBufferString("hello")}
178183
downloader.DownloadReleaseAssetReturns(file, "", nil)
179184

180-
output := bytes.NewBuffer(nil)
181185
grsMock := &component.GithubReleaseSource{
182-
Logger: log.New(output, "[Github release source] ", log.Default().Flags()),
186+
Logger: log.New(GinkgoWriter, "[Github release source] ", log.Default().Flags()),
183187
ReleaseAssetDownloader: downloader,
184188
ReleaseByTagGetter: releaseGetter,
185189
ReleaseSourceConfig: cargo.ReleaseSourceConfig{
@@ -229,7 +233,10 @@ func TestGithubReleaseSource_FindReleaseVersion(t *testing.T) {
229233
s := cargo.BOSHReleaseTarballSpecification{
230234
Version: "garbage",
231235
}
232-
grs := component.NewGithubReleaseSource(cargo.ReleaseSourceConfig{Type: component.ReleaseSourceTypeGithub, GithubToken: "fake_token", Org: "cloudfoundry"})
236+
237+
logger := log.New(GinkgoWriter, "[test] ", log.Default().Flags())
238+
239+
grs := component.NewGithubReleaseSource(cargo.ReleaseSourceConfig{Type: component.ReleaseSourceTypeGithub, GithubToken: "fake_token", Org: "cloudfoundry"}, logger)
233240
_, err := grs.FindReleaseVersion(s, false)
234241

235242
t.Run("it returns an error about version not being specific", func(t *testing.T) {
@@ -273,10 +280,8 @@ func TestGithubReleaseSource_FindReleaseVersion(t *testing.T) {
273280
GitHubRepository: "https://github.com/cloudfoundry/bpm-release",
274281
}
275282

276-
output := bytes.NewBuffer(nil)
277-
defer t.Log(output.String())
278283
grsMock := &component.GithubReleaseSource{
279-
Logger: log.New(output, "[test] ", log.Default().Flags()),
284+
Logger: log.New(GinkgoWriter, "[test] ", log.Default().Flags()),
280285
ReleaseAssetDownloader: downloader,
281286
ReleasesLister: lister,
282287
ReleaseSourceConfig: cargo.ReleaseSourceConfig{
@@ -299,7 +304,10 @@ func TestGithubReleaseSource_GetMatchedRelease(t *testing.T) {
299304
s := cargo.BOSHReleaseTarballSpecification{
300305
Version: ">1.0.0",
301306
}
302-
grs := component.NewGithubReleaseSource(cargo.ReleaseSourceConfig{Type: component.ReleaseSourceTypeGithub, GithubToken: "fake_token", Org: "cloudfoundry"})
307+
308+
logger := log.New(GinkgoWriter, "[test] ", log.Default().Flags())
309+
310+
grs := component.NewGithubReleaseSource(cargo.ReleaseSourceConfig{Type: component.ReleaseSourceTypeGithub, GithubToken: "fake_token", Org: "cloudfoundry"}, logger)
303311
_, err := grs.GetMatchedRelease(s)
304312

305313
t.Run("it returns an error about version not being specific", func(t *testing.T) {
@@ -324,9 +332,8 @@ func TestGetGithubReleaseWithTag(t *testing.T) {
324332

325333
ctx := context.TODO()
326334

327-
output := bytes.NewBuffer(nil)
328335
grsMock := &component.GithubReleaseSource{
329-
Logger: log.New(output, "[test] ", log.Default().Flags()),
336+
Logger: log.New(GinkgoWriter, "[test] ", log.Default().Flags()),
330337
ReleaseByTagGetter: releaseGetter,
331338
ReleaseSourceConfig: cargo.ReleaseSourceConfig{
332339
Type: component.ReleaseSourceTypeGithub,
@@ -365,9 +372,8 @@ func TestGetGithubReleaseWithTag(t *testing.T) {
365372

366373
ctx := context.TODO()
367374

368-
output := bytes.NewBuffer(nil)
369375
grsMock := &component.GithubReleaseSource{
370-
Logger: log.New(output, "[test] ", log.Default().Flags()),
376+
Logger: log.New(GinkgoWriter, "[test] ", log.Default().Flags()),
371377
ReleaseByTagGetter: releaseGetter,
372378
ReleaseSourceConfig: cargo.ReleaseSourceConfig{
373379
Type: component.ReleaseSourceTypeGithub,
@@ -430,9 +436,8 @@ func TestGetLatestMatchingRelease(t *testing.T) {
430436
nil,
431437
)
432438

433-
output := bytes.NewBuffer(nil)
434439
grsMock := &component.GithubReleaseSource{
435-
Logger: log.New(output, "[test] ", log.Default().Flags()),
440+
Logger: log.New(GinkgoWriter, "[test] ", log.Default().Flags()),
436441
ReleasesLister: releaseGetter,
437442
ReleaseSourceConfig: cargo.ReleaseSourceConfig{
438443
Org: "test-org",
@@ -480,9 +485,8 @@ func TestGetLatestMatchingRelease(t *testing.T) {
480485
nil,
481486
)
482487

483-
output := bytes.NewBuffer(nil)
484488
grsMock := &component.GithubReleaseSource{
485-
Logger: log.New(output, "[test] ", log.Default().Flags()),
489+
Logger: log.New(GinkgoWriter, "[test] ", log.Default().Flags()),
486490
ReleasesLister: releaseGetter,
487491
ReleaseSourceConfig: cargo.ReleaseSourceConfig{
488492
Org: "test-org",
@@ -513,9 +517,8 @@ func TestGetLatestMatchingRelease(t *testing.T) {
513517
}
514518
)
515519

516-
output := bytes.NewBuffer(nil)
517520
grsMock := &component.GithubReleaseSource{
518-
Logger: log.New(output, "[test] ", log.Default().Flags()),
521+
Logger: log.New(GinkgoWriter, "[test] ", log.Default().Flags()),
519522
ReleaseSourceConfig: cargo.ReleaseSourceConfig{
520523
Org: githubOrg,
521524
},
@@ -533,11 +536,16 @@ func TestGetLatestMatchingRelease(t *testing.T) {
533536
func TestDownloadReleaseAsset(t *testing.T) {
534537
t.SkipNow()
535538

536-
grs := component.NewGithubReleaseSource(cargo.ReleaseSourceConfig{
537-
Type: component.ReleaseSourceTypeGithub,
538-
GithubToken: os.Getenv("GITHUB_ACCESS_TOKEN"),
539-
Org: "cloudfoundry",
540-
})
539+
logger := log.New(GinkgoWriter, "[test] ", log.Default().Flags())
540+
541+
grs := component.NewGithubReleaseSource(
542+
cargo.ReleaseSourceConfig{
543+
Type: component.ReleaseSourceTypeGithub,
544+
GithubToken: os.Getenv("GITHUB_ACCESS_TOKEN"),
545+
Org: "cloudfoundry",
546+
},
547+
logger,
548+
)
541549
testLock, err := grs.GetMatchedRelease(cargo.BOSHReleaseTarballSpecification{Name: "routing", Version: "0.226.0", GitHubRepository: "https://github.com/cloudfoundry/routing-release"})
542550
if err != nil {
543551
t.Fatal(err)

‎internal/component/release_source.go

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ package component
33
import (
44
"fmt"
55
"io"
6-
"log"
76

87
"github.com/pivotal-cf/kiln/pkg/cargo"
98
)
@@ -81,17 +80,17 @@ const (
8180

8281
// ReleaseSourceFactory returns a configured ReleaseSource based on the Type field on the
8382
// cargo.ReleaseSourceConfig structure.
84-
func ReleaseSourceFactory(releaseConfig cargo.ReleaseSourceConfig, outLogger *log.Logger) ReleaseSource {
83+
func ReleaseSourceFactory(releaseConfig cargo.ReleaseSourceConfig) ReleaseSource {
8584
releaseConfig.ID = cargo.BOSHReleaseTarballSourceID(releaseConfig)
8685
switch releaseConfig.Type {
8786
case ReleaseSourceTypeBOSHIO:
88-
return NewBOSHIOReleaseSource(releaseConfig, "", outLogger)
87+
return NewBOSHIOReleaseSource(releaseConfig, "", nil)
8988
case ReleaseSourceTypeS3:
90-
return NewS3ReleaseSourceFromConfig(releaseConfig, outLogger)
89+
return NewS3ReleaseSourceFromConfig(releaseConfig, nil)
9190
case ReleaseSourceTypeGithub:
92-
return NewGithubReleaseSource(releaseConfig)
91+
return NewGithubReleaseSource(releaseConfig, nil)
9392
case ReleaseSourceTypeArtifactory:
94-
return NewArtifactoryReleaseSource(releaseConfig)
93+
return NewArtifactoryReleaseSource(releaseConfig, nil)
9594
default:
9695
panic(fmt.Sprintf("unknown release config: %v", releaseConfig))
9796
}

‎internal/component/release_source_list.go

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ package component
33
import (
44
"errors"
55
"fmt"
6-
"log"
76

87
"github.com/Masterminds/semver/v3"
98

@@ -12,11 +11,11 @@ import (
1211

1312
type ReleaseSourceList []ReleaseSource
1413

15-
func NewReleaseSourceRepo(kilnfile cargo.Kilnfile, logger *log.Logger) ReleaseSourceList {
14+
func NewReleaseSourceRepo(kilnfile cargo.Kilnfile) ReleaseSourceList {
1615
var list ReleaseSourceList
1716

1817
for _, releaseConfig := range kilnfile.ReleaseSources {
19-
list = append(list, ReleaseSourceFactory(releaseConfig, logger))
18+
list = append(list, ReleaseSourceFactory(releaseConfig))
2019
}
2120

2221
panicIfDuplicateIDs(list)

0 commit comments

Comments
 (0)