Skip to content

gcrhelper: add automatic GCR/Artifact Registry auth on GCE #82

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
jossy wants to merge 1 commit into
base: main
Choose a base branch
from
Open

gcrhelper: add automatic GCR/Artifact Registry auth on GCE #82

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions conversion_helpers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"code.cloudfoundry.org/bbs/models"
"code.cloudfoundry.org/ecrhelper"
"code.cloudfoundry.org/executor"
"code.cloudfoundry.org/gcrhelper"
"code.cloudfoundry.org/routing-info/internalroutes"
)

Expand Down Expand Up @@ -167,6 +168,7 @@ func ConvertPreloadedRootFS(rootFS string, imageLayers []*models.ImageLayer, lay

type RunRequestConversionHelper struct {
ECRHelper ecrhelper.ECRHelper
GCRHelper gcrhelper.GCRHelper
}

func (rrch RunRequestConversionHelper) NewRunRequestFromDesiredLRP(
Expand Down Expand Up @@ -357,6 +359,17 @@ func convertImageLayers(t *models.TaskDefinition) ([]*models.CachedDependency, *
}

func (rrch RunRequestConversionHelper) convertCredentials(rootFS string, username string, password string) (string, string, error) {
if username == "" && password == "" {
isGCRRepo, err := rrch.GCRHelper.IsGCRRepo(rootFS)
if err != nil {
return "", "", err
}

if isGCRRepo {
return rrch.GCRHelper.GetGCRCredentials()
}
}

isECRRepo, err := rrch.ECRHelper.IsECRRepo(rootFS)
if err != nil {
return "", "", err
Expand Down
94 changes: 94 additions & 0 deletions conversion_helpers_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"code.cloudfoundry.org/bbs/test_helpers"
fakeecrhelper "code.cloudfoundry.org/ecrhelper/fakes"
"code.cloudfoundry.org/executor"
fakegcrhelper "code.cloudfoundry.org/gcrhelper/fakes"
"code.cloudfoundry.org/rep"
"code.cloudfoundry.org/routing-info/internalroutes"
. "github.com/onsi/ginkgo/v2"
Expand Down Expand Up @@ -283,12 +284,15 @@ var _ = Describe("Resources", func() {
var (
runRequestConversionHelper rep.RunRequestConversionHelper
fakeECRHelper *fakeecrhelper.FakeECRHelper
fakeGCRHelper *fakegcrhelper.FakeGCRHelper
)

BeforeEach(func() {
fakeECRHelper = &fakeecrhelper.FakeECRHelper{}
fakeGCRHelper = &fakegcrhelper.FakeGCRHelper{}
runRequestConversionHelper = rep.RunRequestConversionHelper{
ECRHelper: fakeECRHelper,
GCRHelper: fakeGCRHelper,
}
})

Expand Down Expand Up @@ -573,6 +577,50 @@ var _ = Describe("Resources", func() {
})
})

Context("when the rootfs is a GCR/Artifact Registry repo URL and no credentials are stored", func() {
BeforeEach(func() {
desiredLRP.ImageUsername = ""
desiredLRP.ImagePassword = ""
fakeGCRHelper.IsGCRRepoReturns(true, nil)
fakeGCRHelper.GetGCRCredentialsReturns("oauth2accesstoken", "some-fresh-token", nil)
})

It("uses the metadata-server token", func() {
runReq, err := runRequestConversionHelper.NewRunRequestFromDesiredLRP(containerGuid, desiredLRP, &actualLRP.ActualLRPKey, &actualLRP.ActualLRPInstanceKey, stackPathMap, rep.LayeringModeSingleLayer)
Expect(err).NotTo(HaveOccurred())
Expect(runReq.ImageUsername).To(Equal("oauth2accesstoken"))
Expect(runReq.ImagePassword).To(Equal("some-fresh-token"))
})

Context("when checking for GCR repo fails", func() {
BeforeEach(func() {
fakeGCRHelper.IsGCRRepoReturns(false, errors.New("disaster"))
})

It("returns an error", func() {
_, err := runRequestConversionHelper.NewRunRequestFromDesiredLRP(containerGuid, desiredLRP, &actualLRP.ActualLRPKey, &actualLRP.ActualLRPInstanceKey, stackPathMap, rep.LayeringModeSingleLayer)
Expect(err).To(HaveOccurred())
})
})

})

Context("when the rootfs is a GCR/Artifact Registry repo URL but credentials are stored", func() {
BeforeEach(func() {
desiredLRP.ImageUsername = "_json_key"
desiredLRP.ImagePassword = `{"type":"service_account"}`
})

It("passes through the stored credentials without checking GCR", func() {
runReq, err := runRequestConversionHelper.NewRunRequestFromDesiredLRP(containerGuid, desiredLRP, &actualLRP.ActualLRPKey, &actualLRP.ActualLRPInstanceKey, stackPathMap, rep.LayeringModeSingleLayer)
Expect(err).NotTo(HaveOccurred())
Expect(runReq.ImageUsername).To(Equal("_json_key"))
Expect(runReq.ImagePassword).To(Equal(`{"type":"service_account"}`))
Expect(fakeGCRHelper.IsGCRRepoCallCount()).To(Equal(0))
Expect(fakeGCRHelper.GetGCRCredentialsCallCount()).To(Equal(0))
})
})

Context("when the rootfs is ECR repo URL", func() {
BeforeEach(func() {
fakeECRHelper.IsECRRepoReturns(true, nil)
Expand Down Expand Up @@ -920,6 +968,52 @@ var _ = Describe("Resources", func() {
})
})

Context("when the rootfs is a GCR/Artifact Registry repo URL and no credentials are stored", func() {
BeforeEach(func() {
task.RootFs = "docker://europe-west3-docker.pkg.dev/my-project/my-repo/my-image:latest"
task.ImageUsername = ""
task.ImagePassword = ""
fakeGCRHelper.IsGCRRepoReturns(true, nil)
fakeGCRHelper.GetGCRCredentialsReturns("oauth2accesstoken", "some-fresh-token", nil)
})

It("uses the metadata-server token", func() {
runReq, err := runRequestConversionHelper.NewRunRequestFromTask(task, stackPathMap, rep.LayeringModeSingleLayer)
Expect(err).NotTo(HaveOccurred())
Expect(runReq.ImageUsername).To(Equal("oauth2accesstoken"))
Expect(runReq.ImagePassword).To(Equal("some-fresh-token"))
})

Context("when checking for GCR repo fails", func() {
BeforeEach(func() {
fakeGCRHelper.IsGCRRepoReturns(false, errors.New("disaster"))
})

It("returns an error", func() {
_, err := runRequestConversionHelper.NewRunRequestFromTask(task, stackPathMap, rep.LayeringModeSingleLayer)
Expect(err).To(HaveOccurred())
})
})

})

Context("when the rootfs is a GCR/Artifact Registry repo URL but credentials are stored", func() {
BeforeEach(func() {
task.RootFs = "docker://europe-west3-docker.pkg.dev/my-project/my-repo/my-image:latest"
task.ImageUsername = "_json_key"
task.ImagePassword = `{"type":"service_account"}`
})

It("passes through the stored credentials without checking GCR", func() {
runReq, err := runRequestConversionHelper.NewRunRequestFromTask(task, stackPathMap, rep.LayeringModeSingleLayer)
Expect(err).NotTo(HaveOccurred())
Expect(runReq.ImageUsername).To(Equal("_json_key"))
Expect(runReq.ImagePassword).To(Equal(`{"type":"service_account"}`))
Expect(fakeGCRHelper.IsGCRRepoCallCount()).To(Equal(0))
Expect(fakeGCRHelper.GetGCRCredentialsCallCount()).To(Equal(0))
})
})

Context("when the rootfs is ECR repo URL", func() {
BeforeEach(func() {
fakeECRHelper.IsECRRepoReturns(true, nil)
Expand Down
6 changes: 5 additions & 1 deletion generator/internal/ordinary_lrp_processor.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"code.cloudfoundry.org/bbs/models"
"code.cloudfoundry.org/ecrhelper"
"code.cloudfoundry.org/executor"
"code.cloudfoundry.org/gcrhelper"
"code.cloudfoundry.org/lager/v3"
"code.cloudfoundry.org/rep"
)
Expand All @@ -27,7 +28,10 @@ func newOrdinaryLRPProcessor(
stackPathMap rep.StackPathMap,
layeringMode string,
) LRPProcessor {
runRequestConversionHelper := rep.RunRequestConversionHelper{ECRHelper: ecrhelper.NewECRHelper()}
runRequestConversionHelper := rep.RunRequestConversionHelper{
ECRHelper: ecrhelper.NewECRHelper(),
GCRHelper: gcrhelper.NewGCRHelper(),
}

return &ordinaryLRPProcessor{
bbsClient: bbsClient,
Expand Down
6 changes: 5 additions & 1 deletion generator/internal/ordinary_lrp_processor_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"code.cloudfoundry.org/bbs/models/test/model_helpers"
fakeecrhelper "code.cloudfoundry.org/ecrhelper/fakes"
"code.cloudfoundry.org/executor"
fakegcrhelper "code.cloudfoundry.org/gcrhelper/fakes"
"code.cloudfoundry.org/lager/v3/lagertest"
"code.cloudfoundry.org/rep"
"code.cloudfoundry.org/rep/evacuation/evacuation_context/fake_evacuation_context"
Expand Down Expand Up @@ -139,7 +140,10 @@ var _ = Describe("OrdinaryLRPProcessor", func() {
It("runs the container", func() {
Expect(containerDelegate.RunContainerCallCount()).To(Equal(1))

runRequestConversionHelper := rep.RunRequestConversionHelper{ECRHelper: &fakeecrhelper.FakeECRHelper{}}
runRequestConversionHelper := rep.RunRequestConversionHelper{
ECRHelper: &fakeecrhelper.FakeECRHelper{},
GCRHelper: &fakegcrhelper.FakeGCRHelper{},
}
expectedRunRequest, err := runRequestConversionHelper.NewRunRequestFromDesiredLRP(container.Guid, desiredLRP, &expectedLrpKey, &expectedInstanceKey, rep.StackPathMap{}, "")
Expect(err).NotTo(HaveOccurred())

Expand Down
6 changes: 5 additions & 1 deletion generator/internal/task_processor.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"code.cloudfoundry.org/bbs/models"
"code.cloudfoundry.org/ecrhelper"
"code.cloudfoundry.org/executor"
"code.cloudfoundry.org/gcrhelper"
"code.cloudfoundry.org/lager/v3"
"code.cloudfoundry.org/rep"
)
Expand All @@ -30,7 +31,10 @@ type taskProcessor struct {
}

func NewTaskProcessor(bbs bbs.InternalClient, containerDelegate ContainerDelegate, cellID string, stackPathMap rep.StackPathMap, layeringMode string) TaskProcessor {
runRequestConversionHelper := rep.RunRequestConversionHelper{ECRHelper: ecrhelper.NewECRHelper()}
runRequestConversionHelper := rep.RunRequestConversionHelper{
ECRHelper: ecrhelper.NewECRHelper(),
GCRHelper: gcrhelper.NewGCRHelper(),
}

return &taskProcessor{
bbsClient: bbs,
Expand Down
6 changes: 5 additions & 1 deletion generator/internal/task_processor_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"code.cloudfoundry.org/bbs/models/test/model_helpers"
fakeecrhelper "code.cloudfoundry.org/ecrhelper/fakes"
"code.cloudfoundry.org/executor"
fakegcrhelper "code.cloudfoundry.org/gcrhelper/fakes"
"code.cloudfoundry.org/lager/v3/lagertest"
"code.cloudfoundry.org/rep"
"code.cloudfoundry.org/rep/generator/internal"
Expand Down Expand Up @@ -42,7 +43,10 @@ var _ = Describe("TaskProcessor", func() {
processor = internal.NewTaskProcessor(bbsClient, containerDelegate, expectedCellID, rep.StackPathMap{}, "")

task = model_helpers.NewValidTask(taskGuid)
runRequestConversionHelper := rep.RunRequestConversionHelper{ECRHelper: &fakeecrhelper.FakeECRHelper{}}
runRequestConversionHelper := rep.RunRequestConversionHelper{
ECRHelper: &fakeecrhelper.FakeECRHelper{},
GCRHelper: &fakegcrhelper.FakeGCRHelper{},
}
expectedRunRequest, err = runRequestConversionHelper.NewRunRequestFromTask(task, rep.StackPathMap{}, "")
Expect(err).NotTo(HaveOccurred())

Expand Down