From 57ebfb4011318a610c13ad82c42e2823a7c6b7a9 Mon Sep 17 00:00:00 2001 From: "Brian C. Lane" Date: Fri, 2 Feb 2024 17:17:49 -0800 Subject: [PATCH] cloudapi: Use distro repos if none included in imageRequest In order to support cloudapi blueprint requests from the cmdline using composer-cli it needs to select the repositories based on the selected distribution instead of requiring the user to include them with the request. If the image request includes repositories they are used, which matches the current behavior. If the repository list is empty it will use the distribution name to select from the repositories shipped with osbuild-composer. --- internal/cloudapi/v2/compose.go | 18 ++++++++++--- internal/cloudapi/v2/compose_test.go | 38 ++++++++++++++++++++++++---- internal/cloudapi/v2/handler.go | 4 +-- internal/cloudapi/v2/v2_test.go | 1 - 4 files changed, 49 insertions(+), 12 deletions(-) diff --git a/internal/cloudapi/v2/compose.go b/internal/cloudapi/v2/compose.go index f31256f86..25f8aff6d 100644 --- a/internal/cloudapi/v2/compose.go +++ b/internal/cloudapi/v2/compose.go @@ -4,16 +4,17 @@ package v2 import ( "crypto/rand" "fmt" - "github.com/osbuild/images/pkg/distrofactory" - "github.com/osbuild/images/pkg/rhsm/facts" - "github.com/osbuild/osbuild-composer/internal/target" "math" "math/big" "reflect" "github.com/osbuild/images/pkg/disk" + "github.com/osbuild/images/pkg/distrofactory" + "github.com/osbuild/images/pkg/reporegistry" + "github.com/osbuild/images/pkg/rhsm/facts" "github.com/osbuild/images/pkg/subscription" "github.com/osbuild/osbuild-composer/internal/blueprint" + "github.com/osbuild/osbuild-composer/internal/target" ) // Return the string representation of the partitioning mode @@ -944,7 +945,7 @@ func (request *ComposeRequest) GetPartitioningMode() (disk.PartitioningMode, err // GetImageRequests converts a composeRequest structure from the API to an intermediate imageRequest structure // that's used for generating manifests and orchestrating worker jobs. -func (request *ComposeRequest) GetImageRequests(distroFactory *distrofactory.Factory) ([]imageRequest, error) { +func (request *ComposeRequest) GetImageRequests(distroFactory *distrofactory.Factory, repoRegistry *reporegistry.RepoRegistry) ([]imageRequest, error) { distribution := distroFactory.GetDistro(request.Distribution) if distribution == nil { return nil, HTTPError(ErrorUnsupportedDistribution) @@ -1002,6 +1003,15 @@ func (request *ComposeRequest) GetImageRequests(distroFactory *distrofactory.Fac return nil, err } + // If no repositories are included with the imageRequest use the defaults for the distro + if len(ir.Repositories) == 0 { + dr, err := repoRegistry.ReposByImageTypeName(request.Distribution, arch.Name(), imageType.Name()) + if err != nil { + return nil, err + } + repos = append(repos, dr...) + } + // Get the initial ImageOptions with image size set imageOptions := ir.GetImageOptions(imageType, bp) diff --git a/internal/cloudapi/v2/compose_test.go b/internal/cloudapi/v2/compose_test.go index 7414a871c..ae7e4b1ee 100644 --- a/internal/cloudapi/v2/compose_test.go +++ b/internal/cloudapi/v2/compose_test.go @@ -1,14 +1,15 @@ package v2 import ( - "github.com/osbuild/images/pkg/distrofactory" - "github.com/osbuild/osbuild-composer/internal/target" "testing" "github.com/osbuild/images/pkg/disk" + "github.com/osbuild/images/pkg/distrofactory" + "github.com/osbuild/images/pkg/reporegistry" "github.com/osbuild/images/pkg/subscription" "github.com/osbuild/osbuild-composer/internal/blueprint" "github.com/osbuild/osbuild-composer/internal/common" + "github.com/osbuild/osbuild-composer/internal/target" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -647,6 +648,7 @@ func TestGetImageRequests_ImageTypeConversion(t *testing.T) { expectedTargetName: target.TargetNameAWSS3, }, } + for _, tt := range tests { t.Run(string(tt.requestedImageType), func(t *testing.T) { for _, d := range tt.requestedDistros { @@ -657,16 +659,42 @@ func TestGetImageRequests_ImageTypeConversion(t *testing.T) { Architecture: "x86_64", ImageType: tt.requestedImageType, UploadOptions: &uo, + Repositories: []Repository{ + Repository{ + Baseurl: common.ToPtr("http://example.org/pub/linux/repo"), + }, + }, }, } - got, err := request.GetImageRequests(distrofactory.NewDefault()) + // NOTE: repoRegistry can be nil as long as ImageRequest.Repositories has data + got, err := request.GetImageRequests(distrofactory.NewDefault(), nil) assert.NoError(t, err) - assert.Len(t, got, 1) + require.Len(t, got, 1) assert.Equal(t, tt.expectedImageType, got[0].imageType.Name()) - assert.Len(t, got[0].targets, 1) + require.Len(t, got[0].targets, 1) assert.Equal(t, tt.expectedTargetName, got[0].targets[0].Name) } }) } } + +func TestGetImageRequests_NoRepositories(t *testing.T) { + uo := UploadOptions(struct{}{}) + request := &ComposeRequest{ + Distribution: "fedora-40", + ImageRequest: &ImageRequest{ + Architecture: "x86_64", + ImageType: ImageTypesAws, + UploadOptions: &uo, + Repositories: []Repository{}, + }, + } + // NOTE: current directory is the location of this file, back up so it can use ./repositories/ + rr, err := reporegistry.New([]string{"../../../"}) + require.NoError(t, err) + got, err := request.GetImageRequests(distrofactory.NewDefault(), rr) + assert.NoError(t, err) + require.Len(t, got, 1) + assert.Greater(t, len(got[0].repositories), 0) +} diff --git a/internal/cloudapi/v2/handler.go b/internal/cloudapi/v2/handler.go index c208a9977..3b567d2b1 100644 --- a/internal/cloudapi/v2/handler.go +++ b/internal/cloudapi/v2/handler.go @@ -4,7 +4,6 @@ package v2 import ( "encoding/json" "fmt" - "github.com/osbuild/osbuild-composer/internal/blueprint" "net/http" "os" "strconv" @@ -17,6 +16,7 @@ import ( "github.com/osbuild/images/pkg/manifest" "github.com/osbuild/images/pkg/osbuild" "github.com/osbuild/images/pkg/rpmmd" + "github.com/osbuild/osbuild-composer/internal/blueprint" "github.com/osbuild/osbuild-composer/internal/common" "github.com/osbuild/osbuild-composer/internal/target" "github.com/osbuild/osbuild-composer/internal/worker" @@ -153,7 +153,7 @@ func (h *apiHandlers) PostCompose(ctx echo.Context) error { return HTTPErrorWithInternal(ErrorTenantNotFound, err) } - irs, err := request.GetImageRequests(h.server.distros) + irs, err := request.GetImageRequests(h.server.distros, h.server.repos) if err != nil { return err } diff --git a/internal/cloudapi/v2/v2_test.go b/internal/cloudapi/v2/v2_test.go index 939287392..6e8db3c23 100644 --- a/internal/cloudapi/v2/v2_test.go +++ b/internal/cloudapi/v2/v2_test.go @@ -35,7 +35,6 @@ func newV2Server(t *testing.T, dir string, depsolveChannels []string, enableJWT distros := distrofactory.NewTestDefault() require.NotNil(t, distros) - // TODO pass it a real path? repos, err := reporegistry.NewTestedDefault() require.Nil(t, err) require.NotNil(t, repos)