From 873c301198390191f1c6987aac5de83dd568ffac Mon Sep 17 00:00:00 2001 From: Bob Fournier Date: Tue, 29 Nov 2022 18:49:01 -0500 Subject: [PATCH] MGMT-12635: Add icsp-file support for all oc commands (#4684) https://github.com/openshift/assisted-service/pull/4141 added support for the --icsp-file parameter to the "oc adm release extract" using the contents of imageContentSources in install-config.yaml. The other oc commands didn't use the --icsp-file option as they used the mirror defined in the env variable OPENSHIFT_RELEASE_IMAGE_MIRROR. Relying on this mirror setting can be problematic if this env variable cannot be set, or if the oc method of deriving the mirror using the --icsp-file is different than the env variable value. This change adds support for the --icsp-file param to the remaining oc commands. The ICSP contents are read from /etc/containers/registries.conf as the install-config.yaml containing the imageContentSources may not be available. (cherry picked from commit 3ade14b5687e857c84f7d155ee813fb321669b30) --- cmd/agentbasedinstaller/register.go | 4 +- cmd/main.go | 4 +- internal/installercache/installercache.go | 4 +- internal/oc/release.go | 150 ++++++++++++++++++++-- internal/oc/release_test.go | 54 +++++++- 5 files changed, 197 insertions(+), 19 deletions(-) diff --git a/cmd/agentbasedinstaller/register.go b/cmd/agentbasedinstaller/register.go index 1f3f7ab3bdf3..cc5e92f30333 100644 --- a/cmd/agentbasedinstaller/register.go +++ b/cmd/agentbasedinstaller/register.go @@ -18,6 +18,7 @@ import ( "github.com/openshift/assisted-service/models" errorutil "github.com/openshift/assisted-service/pkg/error" "github.com/openshift/assisted-service/pkg/executer" + "github.com/openshift/assisted-service/pkg/mirrorregistries" hivev1 "github.com/openshift/hive/apis/hive/v1" "github.com/pkg/errors" log "github.com/sirupsen/logrus" @@ -215,8 +216,9 @@ func getReleaseVersion(clusterImageSetPath string) (string, error) { func getReleaseVersionAndCpuArch(log *log.Logger, releaseImage string, releaseMirror string, pullSecret string) (string, string, error) { // releaseImage is in the form: quay.io:443/openshift-release-dev/ocp-release:4.9.17-x86_64 + mirrorRegistriesBuilder := mirrorregistries.New() releaseHandler := oc.NewRelease(&executer.CommonExecuter{}, - oc.Config{MaxTries: oc.DefaultTries, RetryDelay: oc.DefaltRetryDelay}) + oc.Config{MaxTries: oc.DefaultTries, RetryDelay: oc.DefaltRetryDelay}, mirrorRegistriesBuilder) version, versionError := releaseHandler.GetOpenshiftVersion(log, releaseImage, releaseMirror, pullSecret) if versionError != nil { diff --git a/cmd/main.go b/cmd/main.go index b367fcf0e68c..f966f6ede5be 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -288,8 +288,9 @@ func main() { var k8sClient *kubernetes.Clientset var autoMigrationLeader leader.ElectorInterface + mirrorRegistriesBuilder := mirrorregistries.New() releaseHandler := oc.NewRelease(&executer.CommonExecuter{}, - oc.Config{MaxTries: oc.DefaultTries, RetryDelay: oc.DefaltRetryDelay}) + oc.Config{MaxTries: oc.DefaultTries, RetryDelay: oc.DefaltRetryDelay}, mirrorRegistriesBuilder) extracterHandler := oc.NewExtracter(&executer.CommonExecuter{}, oc.Config{MaxTries: oc.DefaultTries, RetryDelay: oc.DefaltRetryDelay}) versionHandler, err := versions.NewHandler(log.WithField("pkg", "versions"), releaseHandler, @@ -297,7 +298,6 @@ func main() { failOnError(err, "failed to create Versions handler") domainHandler := domains.NewHandler(Options.BMConfig.BaseDNSDomains) staticNetworkConfig := staticnetworkconfig.New(log.WithField("pkg", "static_network_config"), Options.StaticNetworkConfig) - mirrorRegistriesBuilder := mirrorregistries.New() ignitionBuilder, err := ignition.NewBuilder(log.WithField("pkg", "ignition"), staticNetworkConfig, mirrorRegistriesBuilder) failOnError(err, "failed to create ignition builder") installConfigBuilder := installcfg.NewInstallConfigBuilder(log.WithField("pkg", "installcfg"), mirrorRegistriesBuilder, providerRegistry) diff --git a/internal/installercache/installercache.go b/internal/installercache/installercache.go index fd6b6fb90fbe..cf301d27d653 100644 --- a/internal/installercache/installercache.go +++ b/internal/installercache/installercache.go @@ -6,6 +6,7 @@ import ( "github.com/openshift/assisted-service/internal/oc" "github.com/openshift/assisted-service/models" "github.com/openshift/assisted-service/pkg/executer" + "github.com/openshift/assisted-service/pkg/mirrorregistries" "github.com/sirupsen/logrus" ) @@ -48,8 +49,9 @@ func Get(releaseID, releaseIDMirror, cacheDir, pullSecret string, platformType m var err error //cache miss if r.path == "" { + mirrorRegistriesBuilder := mirrorregistries.New() path, err = oc.NewRelease(&executer.CommonExecuter{}, oc.Config{ - MaxTries: oc.DefaultTries, RetryDelay: oc.DefaltRetryDelay}).Extract(log, releaseID, releaseIDMirror, cacheDir, pullSecret, platformType, icspFile) + MaxTries: oc.DefaultTries, RetryDelay: oc.DefaltRetryDelay}, mirrorRegistriesBuilder).Extract(log, releaseID, releaseIDMirror, cacheDir, pullSecret, platformType, icspFile) if err != nil { return "", err } diff --git a/internal/oc/release.go b/internal/oc/release.go index 8c69acf657e3..645b56f28b81 100644 --- a/internal/oc/release.go +++ b/internal/oc/release.go @@ -1,6 +1,7 @@ package oc import ( + "encoding/json" "fmt" "os" "path/filepath" @@ -10,13 +11,17 @@ import ( "github.com/buger/jsonparser" "github.com/hashicorp/go-version" + operatorv1alpha1 "github.com/openshift/api/operator/v1alpha1" "github.com/openshift/assisted-service/internal/common" "github.com/openshift/assisted-service/models" "github.com/openshift/assisted-service/pkg/executer" + "github.com/openshift/assisted-service/pkg/mirrorregistries" "github.com/patrickmn/go-cache" "github.com/pkg/errors" "github.com/sirupsen/logrus" "github.com/thedevsaddam/retry" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + k8syaml "sigs.k8s.io/yaml" ) const ( @@ -49,23 +54,28 @@ type imageValue struct { } type release struct { - executer executer.Executer - config Config + executer executer.Executer + config Config + mirrorRegistriesBuilder mirrorregistries.MirrorRegistriesConfigBuilder // A map for caching images (image name > release image URL > image) imagesMap common.ExpiringCache } -func NewRelease(executer executer.Executer, config Config) Release { - return &release{executer: executer, config: config, imagesMap: common.NewExpiringCache(cache.NoExpiration, cache.NoExpiration)} +func NewRelease(executer executer.Executer, config Config, mirrorRegistriesBuilder mirrorregistries.MirrorRegistriesConfigBuilder) Release { + return &release{executer: executer, config: config, imagesMap: common.NewExpiringCache(cache.NoExpiration, cache.NoExpiration), + mirrorRegistriesBuilder: mirrorRegistriesBuilder} } const ( templateGetImage = "oc adm release info --image-for=%s --insecure=%t %s" + templateGetImageWithIcsp = "oc adm release info --image-for=%s --insecure=%t --icsp-file=%s %s" templateGetVersion = "oc adm release info -o template --template '{{.metadata.version}}' --insecure=%t %s" + templateGetVersionWithIcsp = "oc adm release info -o template --template '{{.metadata.version}}' --insecure=%t --icsp-file=%s %s" templateExtract = "oc adm release extract --command=%s --to=%s --insecure=%t %s" templateExtractWithIcsp = "oc adm release extract --command=%s --to=%s --insecure=%t --icsp-file=%s %s" templateImageInfo = "oc image info --output json %s" + templateImageInfoWithIcsp = "oc image info --output json --icsp-file=%s %s" templateSkopeoDetectMultiarch = "skopeo inspect --raw --no-tags docker://%s" ocAuthArgument = " --registry-config=" skopeoAuthArgument = " --authfile " @@ -94,15 +104,22 @@ func (r *release) getImageByName(log logrus.FieldLogger, imageName, releaseImage if releaseImage == "" && releaseImageMirror == "" { return "", errors.New("neither releaseImage, nor releaseImageMirror are provided") } + + icspFile, err := r.getIcspFileFromRegistriesConfig(log) + if err != nil { + return "", errors.Wrap(err, "failed to create file ICSP file from registries config") + } + defer removeIcspFile(icspFile) + if releaseImageMirror != "" { //TODO: Get mirror registry certificate from install-config - image, err = r.getImageFromRelease(log, imageName, releaseImageMirror, pullSecret, true) + image, err = r.getImageFromRelease(log, imageName, releaseImageMirror, pullSecret, icspFile, true) if err != nil { log.WithError(err).Errorf("failed to get %s image from mirror release image %s", imageName, releaseImageMirror) return "", err } } else { - image, err = r.getImageFromRelease(log, imageName, releaseImage, pullSecret, false) + image, err = r.getImageFromRelease(log, imageName, releaseImage, pullSecret, icspFile, false) if err != nil { log.WithError(err).Errorf("failed to get %s image from release image %s", imageName, releaseImage) return "", err @@ -117,15 +134,22 @@ func (r *release) GetOpenshiftVersion(log logrus.FieldLogger, releaseImage strin if releaseImage == "" && releaseImageMirror == "" { return "", errors.New("no releaseImage nor releaseImageMirror provided") } + + icspFile, err := r.getIcspFileFromRegistriesConfig(log) + if err != nil { + return "", errors.Wrap(err, "failed to create file ICSP file from registries config") + } + defer removeIcspFile(icspFile) + if releaseImageMirror != "" { //TODO: Get mirror registry certificate from install-config - openshiftVersion, err = r.getOpenshiftVersionFromRelease(log, releaseImageMirror, pullSecret, true) + openshiftVersion, err = r.getOpenshiftVersionFromRelease(log, releaseImageMirror, pullSecret, icspFile, true) if err != nil { log.WithError(err).Errorf("failed to get image openshift version from mirror release image %s", releaseImageMirror) return "", err } } else { - openshiftVersion, err = r.getOpenshiftVersionFromRelease(log, releaseImage, pullSecret, false) + openshiftVersion, err = r.getOpenshiftVersionFromRelease(log, releaseImage, pullSecret, icspFile, false) if err != nil { log.WithError(err).Errorf("failed to get image openshift version from release image %s", releaseImage) return "", err @@ -157,7 +181,20 @@ func (r *release) GetReleaseArchitecture(log logrus.FieldLogger, releaseImage st if image == "" { return nil, errors.New("no releaseImage nor releaseImageMirror provided") } - cmd := fmt.Sprintf(templateImageInfo, image) + + icspFile, err := r.getIcspFileFromRegistriesConfig(log) + if err != nil { + return nil, errors.Wrap(err, "failed to create file ICSP file from registries config") + } + defer removeIcspFile(icspFile) + + var cmd string + if icspFile == "" { + cmd = fmt.Sprintf(templateImageInfo, image) + } else { + cmd = fmt.Sprintf(templateImageInfoWithIcsp, icspFile, image) + } + cmdMultiarch := fmt.Sprintf(templateSkopeoDetectMultiarch, image) imageInfoStr, err := execute(log, r.executer, pullSecret, cmd, ocAuthArgument) @@ -222,7 +259,7 @@ func (r *release) getImageValue(imageName, releaseImage string) (*imageValue, er return value, nil } -func (r *release) getImageFromRelease(log logrus.FieldLogger, imageName, releaseImage, pullSecret string, insecure bool) (string, error) { +func (r *release) getImageFromRelease(log logrus.FieldLogger, imageName, releaseImage, pullSecret, icspFile string, insecure bool) (string, error) { // Fetch image URL from cache actualImageValue, err := r.getImageValue(imageName, releaseImage) if err != nil { @@ -237,7 +274,12 @@ func (r *release) getImageFromRelease(log logrus.FieldLogger, imageName, release return actualImageValue.value, nil } - cmd := fmt.Sprintf(templateGetImage, imageName, insecure, releaseImage) + var cmd string + if icspFile == "" { + cmd = fmt.Sprintf(templateGetImage, imageName, insecure, releaseImage) + } else { + cmd = fmt.Sprintf(templateGetImageWithIcsp, imageName, insecure, icspFile, releaseImage) + } log.Infof("Fetching image from OCP release (%s)", cmd) image, err := execute(log, r.executer, pullSecret, cmd, ocAuthArgument) @@ -251,8 +293,13 @@ func (r *release) getImageFromRelease(log logrus.FieldLogger, imageName, release return image, nil } -func (r *release) getOpenshiftVersionFromRelease(log logrus.FieldLogger, releaseImage string, pullSecret string, insecure bool) (string, error) { - cmd := fmt.Sprintf(templateGetVersion, insecure, releaseImage) +func (r *release) getOpenshiftVersionFromRelease(log logrus.FieldLogger, releaseImage, pullSecret, icspFile string, insecure bool) (string, error) { + var cmd string + if icspFile == "" { + cmd = fmt.Sprintf(templateGetVersion, insecure, releaseImage) + } else { + cmd = fmt.Sprintf(templateGetVersionWithIcsp, insecure, icspFile, releaseImage) + } version, err := execute(log, r.executer, pullSecret, cmd, ocAuthArgument) if err != nil { return "", err @@ -351,3 +398,80 @@ func execute(log logrus.FieldLogger, executer executer.Executer, pullSecret stri return "", err } } + +// Create a temporary file containing the ImageContentPolicySources +func (r *release) getIcspFileFromRegistriesConfig(log logrus.FieldLogger) (string, error) { + + if !r.mirrorRegistriesBuilder.IsMirrorRegistriesConfigured() { + log.Debugf("No mirrors configured to build ICSP file") + return "", nil + } + + mirrorRegistriesConfig, err := r.mirrorRegistriesBuilder.ExtractLocationMirrorDataFromRegistries() + if err != nil { + log.WithError(err).Errorf("Failed to get the mirror registries needed for ImageContentSources") + return "", err + } + + contents, err := getIcspContents(mirrorRegistriesConfig) + if err != nil { + log.WithError(err).Errorf("Failed to create the ICSP file from registries.conf") + return "", err + } + if contents == nil { + log.Debugf("No registry entries to build ICSP file") + return "", nil + } + + icspFile, err := os.CreateTemp("", "icsp-file") + if err != nil { + return "", err + } + log.Debugf("Building ICSP file from registries.conf with contents %s", contents) + if _, err := icspFile.Write(contents); err != nil { + icspFile.Close() + os.Remove(icspFile.Name()) + return "", err + } + icspFile.Close() + + return icspFile.Name(), nil +} + +// Convert the data in registries.conf into ICSP format +func getIcspContents(mirrorConfig []mirrorregistries.RegistriesConf) ([]byte, error) { + + icsp := operatorv1alpha1.ImageContentSourcePolicy{ + TypeMeta: metav1.TypeMeta{ + APIVersion: operatorv1alpha1.SchemeGroupVersion.String(), + Kind: "ImageContentSourcePolicy", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "image-policy", + // not namespaced + }, + } + + icsp.Spec.RepositoryDigestMirrors = make([]operatorv1alpha1.RepositoryDigestMirrors, len(mirrorConfig)) + for i, mirrorRegistries := range mirrorConfig { + icsp.Spec.RepositoryDigestMirrors[i] = operatorv1alpha1.RepositoryDigestMirrors{Source: mirrorRegistries.Location, Mirrors: []string{mirrorRegistries.Mirror}} + } + + // Convert to json first so json tags are handled + jsonData, err := json.Marshal(&icsp) + if err != nil { + return nil, err + } + contents, err := k8syaml.JSONToYAML(jsonData) + if err != nil { + return nil, err + } + + return contents, nil +} + +func removeIcspFile(filename string) { + if filename != "" { + os.Remove(filename) + } +} diff --git a/internal/oc/release_test.go b/internal/oc/release_test.go index e645337d43db..5709d2013275 100644 --- a/internal/oc/release_test.go +++ b/internal/oc/release_test.go @@ -15,6 +15,7 @@ import ( "github.com/openshift/assisted-service/internal/common" "github.com/openshift/assisted-service/models" "github.com/openshift/assisted-service/pkg/executer" + "github.com/openshift/assisted-service/pkg/mirrorregistries" logrus "github.com/sirupsen/logrus" ) @@ -46,7 +47,8 @@ var _ = Describe("oc", func() { ctrl = gomock.NewController(GinkgoT()) mockExecuter = executer.NewMockExecuter(ctrl) config := Config{MaxTries: DefaultTries, RetryDelay: time.Millisecond} - oc = NewRelease(mockExecuter, config) + mirrorRegistriesBuilder := mirrorregistries.New() + oc = NewRelease(mockExecuter, config, mirrorRegistriesBuilder) tempFilePath = "/tmp/pull-secret" mockExecuter.EXPECT().TempFile(gomock.Any(), gomock.Any()).DoAndReturn( func(dir, pattern string) (*os.File, error) { @@ -535,7 +537,7 @@ var _ = Describe("getImageFromRelease", func() { } doneChan <- true }() - ret, err := oc.getImageFromRelease(log, r.imageName, r.releaseName, "pull", false) + ret, err := oc.getImageFromRelease(log, r.imageName, r.releaseName, "pull", "", false) Expect(err).ToNot(HaveOccurred()) Expect(ret).To(Equal(r.expectedResult)) }() @@ -557,6 +559,54 @@ var _ = Describe("getImageFromRelease", func() { }) }) +var _ = Describe("getIcspFileFromRegistriesConfig", func() { + var ( + oc *release + mockMirrorRegistriesConfigBuilder *mirrorregistries.MockMirrorRegistriesConfigBuilder + ctrl *gomock.Controller + mockExecuter *executer.MockExecuter + ) + + BeforeEach(func() { + ctrl = gomock.NewController(GinkgoT()) + mockExecuter = executer.NewMockExecuter(ctrl) + mockMirrorRegistriesConfigBuilder = mirrorregistries.NewMockMirrorRegistriesConfigBuilder(ctrl) + config := Config{MaxTries: DefaultTries, RetryDelay: time.Millisecond} + oc = &release{executer: mockExecuter, config: config, imagesMap: common.NewExpiringCache(time.Hour, time.Hour), + mirrorRegistriesBuilder: mockMirrorRegistriesConfigBuilder} + log = logrus.New() + }) + + It("valid_mirror_registries", func() { + regData := []mirrorregistries.RegistriesConf{{Location: "registry.ci.org", Mirror: "host1.example.org:5000/localimages"}, {Location: "quay.io", Mirror: "host1.example.org:5000/localimages"}} + mockMirrorRegistriesConfigBuilder.EXPECT().IsMirrorRegistriesConfigured().Return(true).Times(1) + mockMirrorRegistriesConfigBuilder.EXPECT().ExtractLocationMirrorDataFromRegistries().Return(regData, nil).Times(1) + expected := "apiVersion: operator.openshift.io/v1alpha1\nkind: ImageContentSourcePolicy\nmetadata:\n creationTimestamp: null\n name: image-policy\nspec:\n repositoryDigestMirrors:\n - mirrors:\n - host1.example.org:5000/localimages\n source: registry.ci.org\n - mirrors:\n - host1.example.org:5000/localimages\n source: quay.io\n" + icspFile, err := oc.getIcspFileFromRegistriesConfig(log) + Expect(err).ShouldNot(HaveOccurred()) + Expect(icspFile).ShouldNot(Equal("")) + data, err := os.ReadFile(icspFile) + Expect(err).ShouldNot(HaveOccurred()) + Expect(string(data)).Should(Equal(expected)) + }) + + It("no_registries", func() { + mockMirrorRegistriesConfigBuilder.EXPECT().IsMirrorRegistriesConfigured().Return(false).Times(1) + icspFile, err := oc.getIcspFileFromRegistriesConfig(log) + Expect(err).ShouldNot(HaveOccurred()) + Expect(icspFile).Should(Equal("")) + }) + + It("mirror_registries_invalid", func() { + mockMirrorRegistriesConfigBuilder.EXPECT().IsMirrorRegistriesConfigured().Return(true).Times(1) + mockMirrorRegistriesConfigBuilder.EXPECT().ExtractLocationMirrorDataFromRegistries().Return(nil, fmt.Errorf("extract failed")).Times(1) + icspFile, err := oc.getIcspFileFromRegistriesConfig(log) + Expect(err).Should(HaveOccurred()) + Expect(err).Should(MatchError("extract failed")) + Expect(icspFile).Should(Equal("")) + }) +}) + func splitStringToInterfacesArray(str string) []interface{} { argsAsString := strings.Split(str, " ") argsAsInterface := make([]interface{}, len(argsAsString))