Skip to content

Commit

Permalink
pass context through stack for feature flagging
Browse files Browse the repository at this point in the history
Signed-off-by: Jonathan Holloway <[email protected]>
  • Loading branch information
loadtheaccumulator authored and ezr-ondrej committed Jan 27, 2025
1 parent 0646994 commit 9b45dd7
Show file tree
Hide file tree
Showing 23 changed files with 199 additions and 173 deletions.
8 changes: 4 additions & 4 deletions pkg/clients/imagebuilder/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import (
// ClientInterface is an Interface to make request to ImageBuilder
type ClientInterface interface {
ComposeCommit(image *models.Image) (*models.Image, error)
ComposeInstaller(image *models.Image) (*models.Image, error)
ComposeInstaller(ctx context.Context, image *models.Image) (*models.Image, error)
GetCommitStatus(image *models.Image) (*models.Image, error)
GetInstallerStatus(image *models.Image) (*models.Image, error)
GetMetadata(image *models.Image) (*models.Image, error)
Expand Down Expand Up @@ -345,7 +345,7 @@ func (c *Client) ComposeCommit(image *models.Image) (*models.Image, error) {
}

// ComposeInstaller composes an Installer on ImageBuilder
func (c *Client) ComposeInstaller(image *models.Image) (*models.Image, error) {
func (c *Client) ComposeInstaller(ctx context.Context, image *models.Image) (*models.Image, error) {
c.log.Debug("COMPOSING INSTALLER")

pkgs := make([]string, 0)
Expand All @@ -359,8 +359,8 @@ func (c *Client) ComposeInstaller(image *models.Image) (*models.Image, error) {
rhsm = false
}

if feature.PulpIntegration.IsEnabled() && image.Commit.Repo.PulpURL != "" {
repoURL = image.Commit.Repo.ContentURL()
if feature.PulpIntegration.IsEnabledCtx(ctx) && image.Commit.Repo.ContentURL(ctx) != "" {
repoURL = image.Commit.Repo.ContentURL(ctx)
parsedURL, _ := url.Parse(repoURL)
c.log.WithField("redacted_url", parsedURL.Redacted()).Debug("Using Pulp repo URL for ISO installer request")
}
Expand Down
24 changes: 18 additions & 6 deletions pkg/clients/imagebuilder/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -713,7 +713,9 @@ var _ = Describe("Image Builder Client Test", func() {
},
Installer: &models.Installer{},
}
img, err := client.ComposeInstaller(img)
ctx := context.Background()

img, err := client.ComposeInstaller(ctx, img)
Expect(err).ToNot(HaveOccurred())
Expect(img).ToNot(BeNil())
Expect(img.Installer.ComposeJobID).To(Equal("compose-job-id-returned-from-image-builder"))
Expand Down Expand Up @@ -750,7 +752,9 @@ var _ = Describe("Image Builder Client Test", func() {
Commit: &models.Commit{Arch: "x86_64", Repo: &models.Repo{}},
Installer: &installer,
}
img, err := client.ComposeInstaller(img)

ctx := context.Background()
img, err := client.ComposeInstaller(ctx, img)
Expect(err).ToNot(HaveOccurred())
Expect(img).ToNot(BeNil())
Expect(img.Installer.ComposeJobID).To(Equal(composeJobID))
Expand Down Expand Up @@ -787,7 +791,9 @@ var _ = Describe("Image Builder Client Test", func() {
Commit: &models.Commit{Arch: "x86_64", Repo: &models.Repo{}},
Installer: &installer,
}
img, err := client.ComposeInstaller(img)
ctx := context.Background()

img, err := client.ComposeInstaller(ctx, img)
Expect(err).ToNot(HaveOccurred())
Expect(img).ToNot(BeNil())
Expect(img.Installer.ComposeJobID).To(Equal(composeJobID))
Expand Down Expand Up @@ -831,7 +837,10 @@ var _ = Describe("Image Builder Client Test", func() {
Installer: &installer,
ActivationKey: "test-key",
}
img, err := client.ComposeInstaller(img)

ctx := context.Background()

img, err := client.ComposeInstaller(ctx, img)
Expect(err).ToNot(HaveOccurred())
Expect(img).ToNot(BeNil())
Expect(img.Installer.ComposeJobID).To(Equal(composeJobID))
Expand Down Expand Up @@ -866,7 +875,9 @@ var _ = Describe("Image Builder Client Test", func() {
Commit: &models.Commit{Arch: "x86_64", Repo: &models.Repo{}},
Installer: &installer,
}
img, err := client.ComposeInstaller(img)

ctx := context.Background()
img, err := client.ComposeInstaller(ctx, img)
Expect(err).ToNot(HaveOccurred())
Expect(img).ToNot(BeNil())
Expect(img.Installer.ComposeJobID).To(Equal(composeJobID))
Expand Down Expand Up @@ -999,7 +1010,8 @@ var _ = Describe("Image Builder Client Test", func() {
defer ts.Close()
config.Get().ImageBuilderConfig.URL = ts.URL

image, err := client.ComposeInstaller(&img)
ctx := context.Background()
image, err := client.ComposeInstaller(ctx, &img)
Expect(err).ToNot(HaveOccurred())
Expect(image).ToNot(BeNil())
Expect(image.Installer.ComposeJobID).To(Equal(composeJobID))
Expand Down
9 changes: 5 additions & 4 deletions pkg/clients/imagebuilder/mock_imagebuilder/client.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 5 additions & 4 deletions pkg/models/commits.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
package models

import (
"context"
"errors"
"net/url"

Expand Down Expand Up @@ -69,10 +70,10 @@ type Repo struct {
}

// ContentURL is the URL for internal and Image Builder access to the content in a Pulp repo
func (r Repo) ContentURL() string {
func (r Repo) ContentURL(ctx context.Context) string {
pulpConfig := config.Get().Pulp

if feature.PulpIntegration.IsEnabled() && r.PulpStatus == RepoStatusSuccess {
if feature.PulpIntegration.IsEnabledCtx(ctx) && r.PulpStatus == RepoStatusSuccess {
parsedURL, _ := url.Parse(r.PulpURL)
parsedConfigContentURL, _ := url.Parse(pulpConfig.ContentURL)
parsedURL.Host = parsedConfigContentURL.Host
Expand All @@ -84,8 +85,8 @@ func (r Repo) ContentURL() string {
}

// DistributionURL is the URL for external access to the content in a Pulp repo
func (r Repo) DistributionURL() string {
if feature.PulpIntegration.IsEnabled() && r.PulpStatus == RepoStatusSuccess {
func (r Repo) DistributionURL(ctx context.Context) string {
if feature.PulpIntegration.IsEnabledCtx(ctx) && r.PulpStatus == RepoStatusSuccess {
return r.PulpURL
}

Expand Down
12 changes: 8 additions & 4 deletions pkg/models/commits_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
package models

import (
"context"
"os"
"testing"

Expand Down Expand Up @@ -38,6 +39,7 @@ func TestCommitsBeforeCreate(t *testing.T) {
}

func TestDistributionURL(t *testing.T) {
ctx := context.Background()
var awsURL = "https://aws.repo.example.com/repo/is/here"
var pulpURL = "https://pulp.distribution.example.com/api/pulp-content/pulp/repo/is/here"
repo := Repo{
Expand All @@ -53,7 +55,7 @@ func TestDistributionURL(t *testing.T) {
os.Unsetenv("FEATURE_PULP_INTEGRATION")
os.Unsetenv("PULP_CONTENT_URL")

assert.Equal(t, awsURL, repo.DistributionURL())
assert.Equal(t, awsURL, repo.DistributionURL(ctx))
})

t.Run("return pulp distribution url", func(t *testing.T) {
Expand All @@ -62,11 +64,13 @@ func TestDistributionURL(t *testing.T) {
os.Setenv("FEATURE_PULP_INTEGRATION", "true")
os.Setenv("PULP_CONTENT_URL", "http://internal.repo.example.com:8080")

assert.Equal(t, pulpURL, repo.DistributionURL())
assert.Equal(t, pulpURL, repo.DistributionURL(ctx))
})
}

func TestContentURL(t *testing.T) {
ctx := context.Background()

var awsURL = "https://aws.repo.example.com/repo/is/here"
var pulpURL = "https://pulp.distribution.example.com:3030/api/pulp-content/pulp/repo/is/here"
repo := Repo{
Expand All @@ -84,7 +88,7 @@ func TestContentURL(t *testing.T) {

var expectedURL = "https://internal.repo.example.com:8080/api/pulp-content/pulp/repo/is/here"

assert.Equal(t, expectedURL, repo.ContentURL())
assert.Equal(t, expectedURL, repo.ContentURL(ctx))
})

t.Run("return aws content url", func(t *testing.T) {
Expand All @@ -95,6 +99,6 @@ func TestContentURL(t *testing.T) {

var expectedURL = "https://aws.repo.example.com/repo/is/here"

assert.Equal(t, expectedURL, repo.ContentURL())
assert.Equal(t, expectedURL, repo.ContentURL(ctx))
})
}
2 changes: 1 addition & 1 deletion pkg/routes/devicegroups.go
Original file line number Diff line number Diff line change
Expand Up @@ -880,7 +880,7 @@ func UpdateAllDevicesFromGroup(w http.ResponseWriter, r *http.Request) {
return
}
// should be refactored to avoid performance issue with large volume
updates, err := ctxServices.UpdateService.BuildUpdateTransactions(&devicesUpdate, orgID, commit)
updates, err := ctxServices.UpdateService.BuildUpdateTransactions(r.Context(), &devicesUpdate, orgID, commit)
if err != nil {
ctxServices.Log.WithFields(log.Fields{
"error": err.Error(),
Expand Down
2 changes: 1 addition & 1 deletion pkg/routes/devicegroups_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -823,7 +823,7 @@ var _ = Describe("DeviceGroup routes", func() {
Return(commitID, nil)
mockCommitService.EXPECT().GetCommitByID(commitID, orgID).
Return(&commit, nil)
mockUpdateService.EXPECT().BuildUpdateTransactions(&devicesUpdate, orgID, &commit).
mockUpdateService.EXPECT().BuildUpdateTransactions(ctx, &devicesUpdate, orgID, &commit).
Return(&updTransactions, nil)
for _, trans := range updTransactions {
mockUpdateService.EXPECT().CreateUpdateAsync(trans.ID)
Expand Down
2 changes: 1 addition & 1 deletion pkg/routes/images.go
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,7 @@ func CreateImageUpdate(w http.ResponseWriter, r *http.Request) {
}

ctxServices.Log.Debug("Updating an image from API request")
err = ctxServices.ImageService.UpdateImage(image, previousImage)
err = ctxServices.ImageService.UpdateImage(r.Context(), image, previousImage)
if err != nil {
ctxServices.Log.WithField("error", err.Error()).Error("Failed creating an update to an image")
var apiError errors.APIError
Expand Down
11 changes: 5 additions & 6 deletions pkg/routes/images_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1313,7 +1313,6 @@ var _ = Describe("Images Route Tests", func() {

It("should update image successfully", func() {
Expect(res.Error).ToNot(HaveOccurred())

var buf bytes.Buffer
err := json.NewEncoder(&buf).Encode(&updateImage)
Expect(err).ToNot(HaveOccurred())
Expand All @@ -1324,7 +1323,7 @@ var _ = Describe("Images Route Tests", func() {
mockImagesService.EXPECT().GetImageByID(strconv.Itoa(int(image.ID))).Return(&image, nil)
// we cannot predict the instance value of first argument, we know that it's updateImage
// but as it's un-marshaled it's using an other pointer, most important assertions are those at the end of the test
mockImagesService.EXPECT().UpdateImage(gomock.Any(), &image).Return(nil)
mockImagesService.EXPECT().UpdateImage(gomock.Any(), gomock.Any(), &image).Return(nil)
// same here for context and updateImage
mockImagesService.EXPECT().ProcessImage(gomock.Any(), gomock.Any(), gomock.Any())
httpTestRecorder := httptest.NewRecorder()
Expand Down Expand Up @@ -1356,7 +1355,7 @@ var _ = Describe("Images Route Tests", func() {
// we cannot predict the instance value of first argument, we know that it's updateImage
// but as it's un-marshaled it's using an other pointer, most important assertions are those at the end of the test
expectedErr := new(services.PackageNameDoesNotExist)
mockImagesService.EXPECT().UpdateImage(gomock.Any(), &image).Return(expectedErr)
mockImagesService.EXPECT().UpdateImage(gomock.Any(), gomock.Any(), &image).Return(expectedErr)
httpTestRecorder := httptest.NewRecorder()
router.ServeHTTP(httpTestRecorder, req)

Expand All @@ -1378,7 +1377,7 @@ var _ = Describe("Images Route Tests", func() {
// we cannot predict the instance value of first argument, we know that it's updateImage
// but as it's un-marshaled it's using an other pointer, most important assertions are those at the end of the test
expectedErr := errors.New("unknown error occurred")
mockImagesService.EXPECT().UpdateImage(gomock.Any(), &image).Return(expectedErr)
mockImagesService.EXPECT().UpdateImage(gomock.Any(), gomock.Any(), &image).Return(expectedErr)
httpTestRecorder := httptest.NewRecorder()
router.ServeHTTP(httpTestRecorder, req)

Expand Down Expand Up @@ -1410,7 +1409,7 @@ var _ = Describe("Images Route Tests", func() {
mockImagesService.EXPECT().GetImageByID(strconv.Itoa(int(image.ID))).Return(&image, nil)
// we cannot predict the instance value of first argument, we know that it's updateImage
// but as it's un-marshaled it's using another pointer.
mockImagesService.EXPECT().UpdateImage(gomock.AssignableToTypeOf(&updateImage), &image).Return(nil)
mockImagesService.EXPECT().UpdateImage(gomock.Any(), gomock.AssignableToTypeOf(&updateImage), &image).Return(nil)
// same here for context and updateImage
mockImagesService.EXPECT().ProcessImage(gomock.Any(), gomock.AssignableToTypeOf(&updateImage), gomock.Any())

Expand Down Expand Up @@ -1454,7 +1453,7 @@ var _ = Describe("Images Route Tests", func() {
mockImagesService.EXPECT().GetImageByID(strconv.Itoa(int(image.ID))).Return(&image, nil)
// we cannot predict the instance value of first argument, we know that it's updateImage
// but as it's un-marshaled it's using another pointer.
mockImagesService.EXPECT().UpdateImage(gomock.AssignableToTypeOf(&updateImage), &image).Return(new(services.ImageNameChangeIsProhibited))
mockImagesService.EXPECT().UpdateImage(gomock.Any(), gomock.AssignableToTypeOf(&updateImage), &image).Return(new(services.ImageNameChangeIsProhibited))

httpTestRecorder := httptest.NewRecorder()
router.ServeHTTP(httpTestRecorder, req)
Expand Down
6 changes: 3 additions & 3 deletions pkg/routes/storage.go
Original file line number Diff line number Diff line change
Expand Up @@ -517,17 +517,17 @@ func ValidateStorageImage(w http.ResponseWriter, r *http.Request) string {
return ""
}

if image.Commit.Repo == nil || image.Commit.Repo.DistributionURL() == "" {
if image.Commit.Repo == nil || image.Commit.Repo.DistributionURL(r.Context()) == "" {
logger.Error("image repository does not exist")
respondWithAPIError(w, logger, errors.NewNotFound("image repository does not exist"))
return ""
}

RepoURL, err := url2.Parse(image.Commit.Repo.DistributionURL())
RepoURL, err := url2.Parse(image.Commit.Repo.DistributionURL(r.Context()))
if err != nil {
logger.WithFields(log.Fields{
"error": err.Error(),
"URL": image.Commit.Repo.DistributionURL(),
"URL": image.Commit.Repo.DistributionURL(r.Context()),
}).Error("error occurred when parsing repository url")
respondWithAPIError(w, logger, errors.NewBadRequest("bad image repository url"))
return ""
Expand Down
2 changes: 1 addition & 1 deletion pkg/routes/updates.go
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,7 @@ func updateFromHTTP(w http.ResponseWriter, r *http.Request) *[]models.UpdateTran
}
}
ctxServices.Log.WithField("commit", commit.ID).Debug("Commit retrieved from this update")
updates, err := ctxServices.UpdateService.BuildUpdateTransactions(&devicesUpdate, orgID, commit)
updates, err := ctxServices.UpdateService.BuildUpdateTransactions(r.Context(), &devicesUpdate, orgID, commit)
if err != nil {
ctxServices.Log.WithFields(log.Fields{
"error": err.Error(),
Expand Down
10 changes: 5 additions & 5 deletions pkg/routes/updates_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -385,7 +385,7 @@ var _ = Describe("Update routes", func() {
ctx = dependencies.ContextWithServices(ctx, edgeAPIServices)
req = req.WithContext(ctx)

mockUpdateService.EXPECT().BuildUpdateTransactions(gomock.Any(), orgID, gomock.Any()).Return(&[]models.UpdateTransaction{}, nil)
mockUpdateService.EXPECT().BuildUpdateTransactions(ctx, gomock.Any(), orgID, gomock.Any()).Return(&[]models.UpdateTransaction{}, nil)

rr := httptest.NewRecorder()
handler := http.HandlerFunc(AddUpdate)
Expand Down Expand Up @@ -433,7 +433,7 @@ var _ = Describe("Update routes", func() {
ctx = dependencies.ContextWithServices(ctx, edgeAPIServices)
req = req.WithContext(ctx)

mockUpdateService.EXPECT().BuildUpdateTransactions(gomock.Any(), orgID, gomock.Any()).Return(&updateTransactions, nil)
mockUpdateService.EXPECT().BuildUpdateTransactions(ctx, gomock.Any(), orgID, gomock.Any()).Return(&updateTransactions, nil)
mockUpdateService.EXPECT().CreateUpdateAsync(updateTransactions[0].ID)

responseRecorder := httptest.NewRecorder()
Expand All @@ -453,7 +453,7 @@ var _ = Describe("Update routes", func() {
ctx = dependencies.ContextWithServices(ctx, edgeAPIServices)
req = req.WithContext(ctx)

mockUpdateService.EXPECT().BuildUpdateTransactions(gomock.Any(), orgID, gomock.Any()).Return(&updateTransactions, nil)
mockUpdateService.EXPECT().BuildUpdateTransactions(ctx, gomock.Any(), orgID, gomock.Any()).Return(&updateTransactions, nil)
mockUpdateService.EXPECT().CreateUpdateAsync(updateTransactions[0].ID)

responseRecorder := httptest.NewRecorder()
Expand Down Expand Up @@ -632,7 +632,7 @@ var _ = Describe("Update routes", func() {
req = req.WithContext(ctx)
var desiredCommit models.Commit
db.DB.First(&desiredCommit, &commits[2].ID)
mockUpdateService.EXPECT().BuildUpdateTransactions(&models.DevicesUpdate{DevicesUUID: []string{device.UUID}, CommitID: commits[2].ID},
mockUpdateService.EXPECT().BuildUpdateTransactions(ctx, &models.DevicesUpdate{DevicesUUID: []string{device.UUID}, CommitID: commits[2].ID},
orgID, &desiredCommit).
Return(&[]models.UpdateTransaction{}, nil)
rr := httptest.NewRecorder()
Expand All @@ -658,7 +658,7 @@ var _ = Describe("Update routes", func() {
req = req.WithContext(ctx)
var desiredCommit models.Commit
db.DB.First(&desiredCommit, &commits[3].ID)
mockUpdateService.EXPECT().BuildUpdateTransactions(&models.DevicesUpdate{DevicesUUID: []string{device.UUID}, CommitID: commits[3].ID},
mockUpdateService.EXPECT().BuildUpdateTransactions(ctx, &models.DevicesUpdate{DevicesUUID: []string{device.UUID}, CommitID: commits[3].ID},
orgID, &desiredCommit).
Return(&[]models.UpdateTransaction{}, nil)

Expand Down
Loading

0 comments on commit 9b45dd7

Please sign in to comment.