Fix a bug in the GitHub release source

internal/commands/update_release.go

@@ -89,7 +89,6 @@ func (u UpdateRelease) Execute(args []string) error {
 		newSHA1 = remoteRelease.SHA1
 		newSourceID = remoteRelease.RemoteSource
 		newRemotePath = remoteRelease.RemotePath
-
 	} else {
 		remoteRelease, err = releaseSource.GetMatchedRelease(cargo.BOSHReleaseTarballSpecification{
 			Name:             u.Options.Name,

internal/component/artifactory.go → internal/component/artifactory_release_source.go

@@ -20,6 +20,7 @@ import (
 	"text/template"
 
 	"github.com/Masterminds/semver/v3"
+
 	"github.com/pivotal-cf/kiln/pkg/cargo"
 )
 
@@ -53,18 +54,22 @@ type ArtifactoryFileInfo struct {
 
 // NewArtifactoryReleaseSource will provision a new ArtifactoryReleaseSource Project
 // from the Kilnfile (ReleaseSourceConfig). If type is incorrect it will PANIC
-func NewArtifactoryReleaseSource(c cargo.ReleaseSourceConfig) *ArtifactoryReleaseSource {
+func NewArtifactoryReleaseSource(c cargo.ReleaseSourceConfig, logger *log.Logger) *ArtifactoryReleaseSource {
 	if c.Type != "" && c.Type != ReleaseSourceTypeArtifactory {
 		panic(panicMessageWrongReleaseSourceType)
 	}
 
 	// ctx := context.TODO()
 
+	if logger == nil {
+		logger = log.New(os.Stderr, "[Artifactory release source] ", log.Default().Flags())
+	}
+
 	return &ArtifactoryReleaseSource{
 		Client:              http.DefaultClient,
 		ReleaseSourceConfig: c,
 		ID:                  c.ID,
-		logger:              log.New(os.Stderr, "[Artifactory release source] ", log.Default().Flags()),
+		logger:              logger,
 	}
 }
 

internal/component/artifactory_test.go → internal/component/artifactory_release_source_test.go

@@ -54,9 +54,10 @@ var _ = Describe("interacting with BOSH releases on Artifactory", func() {
 		})
 	})
 	JustBeforeEach(func() {
+		logger := log.New(GinkgoWriter, "", 0)
 		server = httptest.NewServer(artifactoryRouter)
 		config.ArtifactoryHost = server.URL
-		source = component.NewArtifactoryReleaseSource(config)
+		source = component.NewArtifactoryReleaseSource(config, logger)
 		source.Client = server.Client()
 	})
 	AfterEach(func() {
@@ -142,8 +143,9 @@ var _ = Describe("interacting with BOSH releases on Artifactory", func() {
 			})
 			When("the server URL ends in /artifactory", func() {
 				JustBeforeEach(func() {
+					logger := log.New(GinkgoWriter, "", 0)
 					config.ArtifactoryHost = server.URL + "/artifactory"
-					source = component.NewArtifactoryReleaseSource(config)
+					source = component.NewArtifactoryReleaseSource(config, logger)
 					source.Client = server.Client()
 				})
 
@@ -162,8 +164,9 @@ var _ = Describe("interacting with BOSH releases on Artifactory", func() {
 			})
 			When("the server URL is malformed", func() {
 				JustBeforeEach(func() {
+					logger := log.New(GinkgoWriter, "", 0)
 					config.ArtifactoryHost = ":improper-url/formatting"
-					source = component.NewArtifactoryReleaseSource(config)
+					source = component.NewArtifactoryReleaseSource(config, logger)
 					source.Client = server.Client()
 				})
 				It("returns an error", func() {

internal/component/bosh_io_release_source.go

@@ -12,6 +12,7 @@ import (
 	"path/filepath"
 
 	"github.com/Masterminds/semver/v3"
+
 	"github.com/pivotal-cf/kiln/pkg/cargo"
 )
 
@@ -61,9 +62,11 @@ func NewBOSHIOReleaseSource(c cargo.ReleaseSourceConfig, customServerURI string,
 	if customServerURI == "" {
 		customServerURI = "https://bosh.io"
 	}
+
 	if logger == nil {
 		logger = log.New(os.Stderr, "[bosh.io release source] ", log.Default().Flags())
 	}
+
 	return &BOSHIOReleaseSource{
 		ReleaseSourceConfig: c,
 		logger:              logger,

internal/component/github_release_source.go

@@ -33,13 +33,15 @@ type GithubReleaseSource struct {
 
 // NewGithubReleaseSource will provision a new GithubReleaseSource Project
 // from the Kilnfile (ReleaseSourceConfig). If type is incorrect it will PANIC
-func NewGithubReleaseSource(c cargo.ReleaseSourceConfig) *GithubReleaseSource {
+func NewGithubReleaseSource(c cargo.ReleaseSourceConfig, logger *log.Logger) *GithubReleaseSource {
 	if c.Type != "" && c.Type != ReleaseSourceTypeGithub {
 		panic(panicMessageWrongReleaseSourceType)
 	}
+
 	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")
 	}
@@ -53,14 +55,18 @@ func NewGithubReleaseSource(c cargo.ReleaseSourceConfig) *GithubReleaseSource {
 		host = "https://github.gwd.broadcom.net"
 	}
 
+	if logger == nil {
+		logger = log.New(os.Stderr, "[Github release source] ", log.Default().Flags())
+	}
+
 	githubClient, err := gh.GitClient(context.TODO(), host, c.GithubToken, c.GithubToken)
 	if err != nil {
 		panic(err)
 	}
 	return &GithubReleaseSource{
 		ReleaseSourceConfig: c,
 		Token:               c.GithubToken,
-		Logger:              log.New(os.Stderr, "[Github release source] ", log.Default().Flags()),
+		Logger:              logger,
 
 		ReleaseAssetDownloader: githubClient.Repositories,
 		ReleaseByTagGetter:     githubClient.Repositories,
@@ -103,6 +109,11 @@ func (grs *GithubReleaseSource) GetGithubReleaseWithTag(ctx context.Context, s c
 		return nil, ErrNotFound
 	}
 
+	if repoOwner != grs.Org {
+		grs.Logger.Printf("GitHubRepository owner %q does not match configured Org %q, skipping...", repoOwner, grs.Org)
+		return nil, ErrNotFound
+	}
+
 	release, response, err := grs.GetReleaseByTag(ctx, repoOwner, repoName, "v"+s.Version)
 	if err == nil {
 		err = checkStatus(http.StatusOK, response.StatusCode)

internal/component/github_release_source_test.go

@@ -12,6 +12,7 @@ import (
 	"testing"
 
 	"github.com/google/go-github/v50/github"
+	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
 
 	"github.com/pivotal-cf/kiln/internal/component"
@@ -22,11 +23,16 @@ import (
 func TestListAllOfTheCrap(t *testing.T) {
 	t.SkipNow()
 
-	grs := component.NewGithubReleaseSource(cargo.ReleaseSourceConfig{
-		Type:        component.ReleaseSourceTypeGithub,
-		GithubToken: os.Getenv("GITHUB_ACCESS_TOKEN"),
-		Org:         "cloudfoundry",
-	})
+	logger := log.New(GinkgoWriter, "[test] ", log.Default().Flags())
+
+	grs := component.NewGithubReleaseSource(
+		cargo.ReleaseSourceConfig{
+			Type:        component.ReleaseSourceTypeGithub,
+			GithubToken: os.Getenv("GITHUB_ACCESS_TOKEN"),
+			Org:         "cloudfoundry",
+		},
+		logger,
+	)
 	// grs.ListAllOfTheCrap(context.TODO(), "cloudfoundry")
 
 	// grs.Client.Repositories.GetReleaseByTag()
@@ -88,9 +94,8 @@ func TestGithubReleaseSource_ComponentLockFromGithubRelease(t *testing.T) {
 		file := &SetTrueOnClose{Reader: bytes.NewBufferString("hello")}
 		downloader.DownloadReleaseAssetReturns(file, "", nil)
 
-		output := bytes.NewBuffer(nil)
 		grsMock := &component.GithubReleaseSource{
-			Logger:                 log.New(output, "[Github release source] ", log.Default().Flags()),
+			Logger:                 log.New(GinkgoWriter, "[Github release source] ", log.Default().Flags()),
 			ReleaseAssetDownloader: downloader,
 			ReleaseByTagGetter:     releaseGetter,
 			ReleaseSourceConfig: cargo.ReleaseSourceConfig{
@@ -177,9 +182,8 @@ func TestGithubReleaseSource_ComponentLockFromGithubRelease(t *testing.T) {
 		file := &SetTrueOnClose{Reader: bytes.NewBufferString("hello")}
 		downloader.DownloadReleaseAssetReturns(file, "", nil)
 
-		output := bytes.NewBuffer(nil)
 		grsMock := &component.GithubReleaseSource{
-			Logger:                 log.New(output, "[Github release source] ", log.Default().Flags()),
+			Logger:                 log.New(GinkgoWriter, "[Github release source] ", log.Default().Flags()),
 			ReleaseAssetDownloader: downloader,
 			ReleaseByTagGetter:     releaseGetter,
 			ReleaseSourceConfig: cargo.ReleaseSourceConfig{
@@ -229,7 +233,10 @@ func TestGithubReleaseSource_FindReleaseVersion(t *testing.T) {
 		s := cargo.BOSHReleaseTarballSpecification{
 			Version: "garbage",
 		}
-		grs := component.NewGithubReleaseSource(cargo.ReleaseSourceConfig{Type: component.ReleaseSourceTypeGithub, GithubToken: "fake_token", Org: "cloudfoundry"})
+
+		logger := log.New(GinkgoWriter, "[test] ", log.Default().Flags())
+
+		grs := component.NewGithubReleaseSource(cargo.ReleaseSourceConfig{Type: component.ReleaseSourceTypeGithub, GithubToken: "fake_token", Org: "cloudfoundry"}, logger)
 		_, err := grs.FindReleaseVersion(s, false)
 
 		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) {
 			GitHubRepository: "https://github.com/cloudfoundry/bpm-release",
 		}
 
-		output := bytes.NewBuffer(nil)
-		defer t.Log(output.String())
 		grsMock := &component.GithubReleaseSource{
-			Logger:                 log.New(output, "[test] ", log.Default().Flags()),
+			Logger:                 log.New(GinkgoWriter, "[test] ", log.Default().Flags()),
 			ReleaseAssetDownloader: downloader,
 			ReleasesLister:         lister,
 			ReleaseSourceConfig: cargo.ReleaseSourceConfig{
@@ -299,7 +304,10 @@ func TestGithubReleaseSource_GetMatchedRelease(t *testing.T) {
 		s := cargo.BOSHReleaseTarballSpecification{
 			Version: ">1.0.0",
 		}
-		grs := component.NewGithubReleaseSource(cargo.ReleaseSourceConfig{Type: component.ReleaseSourceTypeGithub, GithubToken: "fake_token", Org: "cloudfoundry"})
+
+		logger := log.New(GinkgoWriter, "[test] ", log.Default().Flags())
+
+		grs := component.NewGithubReleaseSource(cargo.ReleaseSourceConfig{Type: component.ReleaseSourceTypeGithub, GithubToken: "fake_token", Org: "cloudfoundry"}, logger)
 		_, err := grs.GetMatchedRelease(s)
 
 		t.Run("it returns an error about version not being specific", func(t *testing.T) {
@@ -310,8 +318,54 @@ func TestGithubReleaseSource_GetMatchedRelease(t *testing.T) {
 	})
 }
 
-func TestGetGithubReleaseWithTag(t *testing.T) {
-	t.Run("when get release with tag api request fails", func(t *testing.T) {
+func TestGithubReleaseSource_GetGithubReleaseWithTag(t *testing.T) {
+	t.Run("when RepositoryOwnerAndNameFromPath fails", func(t *testing.T) {
+		damnIt := NewWithT(t)
+
+		ctx := context.TODO()
+
+		grsMock := &component.GithubReleaseSource{
+			Logger: log.New(GinkgoWriter, "[test] ", log.Default().Flags()),
+			ReleaseSourceConfig: cargo.ReleaseSourceConfig{
+				Type:        component.ReleaseSourceTypeGithub,
+				Org:         "cloudfoundry",
+				GithubToken: "fake-token",
+			},
+		}
+		s := cargo.BOSHReleaseTarballSpecification{
+			Name:             "routing",
+			Version:          "0.226.0",
+			GitHubRepository: "invalid-uri",
+		}
+
+		_, err := grsMock.GetGithubReleaseWithTag(ctx, s)
+		damnIt.Expect(err).To(MatchError(component.ErrNotFound))
+	})
+
+	t.Run("when the GitHubRepository owner does not match the configured Org", func(t *testing.T) {
+		damnIt := NewWithT(t)
+
+		ctx := context.TODO()
+
+		grsMock := &component.GithubReleaseSource{
+			Logger: log.New(GinkgoWriter, "[test] ", log.Default().Flags()),
+			ReleaseSourceConfig: cargo.ReleaseSourceConfig{
+				Type:        component.ReleaseSourceTypeGithub,
+				Org:         "cloudnotfoundry",
+				GithubToken: "fake-token",
+			},
+		}
+		s := cargo.BOSHReleaseTarballSpecification{
+			Name:             "routing",
+			Version:          "0.226.0",
+			GitHubRepository: "https://github.com/cloudfoundry/routing-release",
+		}
+
+		_, err := grsMock.GetGithubReleaseWithTag(ctx, s)
+		damnIt.Expect(err).To(MatchError(component.ErrNotFound))
+	})
+
+	t.Run("when GetReleaseByTag fails", func(t *testing.T) {
 		damnIt := NewWithT(t)
 
 		releaseGetter := new(fakes.ReleaseByTagGetter)
@@ -324,9 +378,8 @@ func TestGetGithubReleaseWithTag(t *testing.T) {
 
 		ctx := context.TODO()
 
-		output := bytes.NewBuffer(nil)
 		grsMock := &component.GithubReleaseSource{
-			Logger:             log.New(output, "[test] ", log.Default().Flags()),
+			Logger:             log.New(GinkgoWriter, "[test] ", log.Default().Flags()),
 			ReleaseByTagGetter: releaseGetter,
 			ReleaseSourceConfig: cargo.ReleaseSourceConfig{
 				Type:        component.ReleaseSourceTypeGithub,
@@ -365,9 +418,8 @@ func TestGetGithubReleaseWithTag(t *testing.T) {
 
 		ctx := context.TODO()
 
-		output := bytes.NewBuffer(nil)
 		grsMock := &component.GithubReleaseSource{
-			Logger:             log.New(output, "[test] ", log.Default().Flags()),
+			Logger:             log.New(GinkgoWriter, "[test] ", log.Default().Flags()),
 			ReleaseByTagGetter: releaseGetter,
 			ReleaseSourceConfig: cargo.ReleaseSourceConfig{
 				Type:        component.ReleaseSourceTypeGithub,
@@ -430,9 +482,8 @@ func TestGetLatestMatchingRelease(t *testing.T) {
 			nil,
 		)
 
-		output := bytes.NewBuffer(nil)
 		grsMock := &component.GithubReleaseSource{
-			Logger:         log.New(output, "[test] ", log.Default().Flags()),
+			Logger:         log.New(GinkgoWriter, "[test] ", log.Default().Flags()),
 			ReleasesLister: releaseGetter,
 			ReleaseSourceConfig: cargo.ReleaseSourceConfig{
 				Org: "test-org",
@@ -480,9 +531,8 @@ func TestGetLatestMatchingRelease(t *testing.T) {
 			nil,
 		)
 
-		output := bytes.NewBuffer(nil)
 		grsMock := &component.GithubReleaseSource{
-			Logger:         log.New(output, "[test] ", log.Default().Flags()),
+			Logger:         log.New(GinkgoWriter, "[test] ", log.Default().Flags()),
 			ReleasesLister: releaseGetter,
 			ReleaseSourceConfig: cargo.ReleaseSourceConfig{
 				Org: "test-org",
@@ -513,9 +563,8 @@ func TestGetLatestMatchingRelease(t *testing.T) {
 			}
 		)
 
-		output := bytes.NewBuffer(nil)
 		grsMock := &component.GithubReleaseSource{
-			Logger: log.New(output, "[test] ", log.Default().Flags()),
+			Logger: log.New(GinkgoWriter, "[test] ", log.Default().Flags()),
 			ReleaseSourceConfig: cargo.ReleaseSourceConfig{
 				Org: githubOrg,
 			},
@@ -533,11 +582,16 @@ func TestGetLatestMatchingRelease(t *testing.T) {
 func TestDownloadReleaseAsset(t *testing.T) {
 	t.SkipNow()
 
-	grs := component.NewGithubReleaseSource(cargo.ReleaseSourceConfig{
-		Type:        component.ReleaseSourceTypeGithub,
-		GithubToken: os.Getenv("GITHUB_ACCESS_TOKEN"),
-		Org:         "cloudfoundry",
-	})
+	logger := log.New(GinkgoWriter, "[test] ", log.Default().Flags())
+
+	grs := component.NewGithubReleaseSource(
+		cargo.ReleaseSourceConfig{
+			Type:        component.ReleaseSourceTypeGithub,
+			GithubToken: os.Getenv("GITHUB_ACCESS_TOKEN"),
+			Org:         "cloudfoundry",
+		},
+		logger,
+	)
 	testLock, err := grs.GetMatchedRelease(cargo.BOSHReleaseTarballSpecification{Name: "routing", Version: "0.226.0", GitHubRepository: "https://github.com/cloudfoundry/routing-release"})
 	if err != nil {
 		t.Fatal(err)

internal/component/release_source.go

@@ -3,7 +3,6 @@ package component
 import (
 	"fmt"
 	"io"
-	"log"
 
 	"github.com/pivotal-cf/kiln/pkg/cargo"
 )
@@ -81,17 +80,17 @@ const (
 
 // ReleaseSourceFactory returns a configured ReleaseSource based on the Type field on the
 // cargo.ReleaseSourceConfig structure.
-func ReleaseSourceFactory(releaseConfig cargo.ReleaseSourceConfig, outLogger *log.Logger) ReleaseSource {
+func ReleaseSourceFactory(releaseConfig cargo.ReleaseSourceConfig) ReleaseSource {
 	releaseConfig.ID = cargo.BOSHReleaseTarballSourceID(releaseConfig)
 	switch releaseConfig.Type {
 	case ReleaseSourceTypeBOSHIO:
-		return NewBOSHIOReleaseSource(releaseConfig, "", outLogger)
+		return NewBOSHIOReleaseSource(releaseConfig, "", nil)
 	case ReleaseSourceTypeS3:
-		return NewS3ReleaseSourceFromConfig(releaseConfig, outLogger)
+		return NewS3ReleaseSourceFromConfig(releaseConfig, nil)
 	case ReleaseSourceTypeGithub:
-		return NewGithubReleaseSource(releaseConfig)
+		return NewGithubReleaseSource(releaseConfig, nil)
 	case ReleaseSourceTypeArtifactory:
-		return NewArtifactoryReleaseSource(releaseConfig)
+		return NewArtifactoryReleaseSource(releaseConfig, nil)
 	default:
 		panic(fmt.Sprintf("unknown release config: %v", releaseConfig))
 	}