From 1db8802da635601129d4ee48086e6967bb52a76b Mon Sep 17 00:00:00 2001 From: Zane Bitter Date: Tue, 5 Jul 2022 16:24:44 -0400 Subject: [PATCH 1/8] Generate CA Bundle from InstallConfig if available --- pkg/asset/agent/mirror/cabundle.go | 38 +++--- pkg/asset/agent/mirror/cabundle_test.go | 121 ++++++++++++++++++ .../manifests/additionaltrustbundleconfig.go | 5 +- 3 files changed, 147 insertions(+), 17 deletions(-) diff --git a/pkg/asset/agent/mirror/cabundle.go b/pkg/asset/agent/mirror/cabundle.go index 0d6a47addca..1cd43c974a4 100644 --- a/pkg/asset/agent/mirror/cabundle.go +++ b/pkg/asset/agent/mirror/cabundle.go @@ -7,6 +7,7 @@ import ( "github.com/openshift/installer/pkg/asset" "github.com/openshift/installer/pkg/asset/agent" + "github.com/openshift/installer/pkg/asset/manifests" "github.com/pkg/errors" ) @@ -37,23 +38,30 @@ func (*CaBundle) Dependencies() []asset.Asset { // Generate generates the Mirror Registries certificate file from install-config. func (i *CaBundle) Generate(dependencies asset.Parents) error { + installConfig := &agent.OptionalInstallConfig{} + dependencies.Get(installConfig) + if !installConfig.Supplied { + return nil + } - // installConfig := &agent.OptionalInstallConfig{} - // dependencies.Get(installConfig) - - // if installConfig.Config.AdditionalTrustBundle == "" { - // return nil - // } - // data, err := parseCertificates(installConfig.Config.AdditionalTrustBundle) - - //if err != nil { - // return err - //} + if installConfig.Config.AdditionalTrustBundle == "" { + return nil + } + data, err := manifests.ParseCertificates(installConfig.Config.AdditionalTrustBundle) + if err != nil { + return err + } - // i.File = &asset.File{ - // Filename: CaBundleFilename, - // Data: data, - // } + for filename, content := range data { + if filepath.Base(CaBundleFilename) == filename { + i.File = &asset.File{ + Filename: CaBundleFilename, + Data: []byte(content), + } + } else { + return fmt.Errorf("unexpected CA Bundle filename %s", filename) + } + } return nil } diff --git a/pkg/asset/agent/mirror/cabundle_test.go b/pkg/asset/agent/mirror/cabundle_test.go index 48006880610..52479fc9d97 100644 --- a/pkg/asset/agent/mirror/cabundle_test.go +++ b/pkg/asset/agent/mirror/cabundle_test.go @@ -7,11 +7,132 @@ import ( "github.com/golang/mock/gomock" "github.com/stretchr/testify/assert" + v1 "k8s.io/apimachinery/pkg/apis/meta/v1" "github.com/openshift/installer/pkg/asset" + "github.com/openshift/installer/pkg/asset/agent" + "github.com/openshift/installer/pkg/asset/installconfig" "github.com/openshift/installer/pkg/asset/mock" + "github.com/openshift/installer/pkg/types" ) +func TestCaBundle_Generate(t *testing.T) { + + cases := []struct { + name string + dependencies []asset.Asset + expectedError string + expectedConfig string + }{ + { + name: "missing-config", + dependencies: []asset.Asset{ + &agent.OptionalInstallConfig{}, + }, + }, + { + name: "default", + dependencies: []asset.Asset{ + &agent.OptionalInstallConfig{ + Supplied: true, + InstallConfig: installconfig.InstallConfig{ + Config: &types.InstallConfig{ + ObjectMeta: v1.ObjectMeta{ + Namespace: "cluster-0", + }, + }, + }, + }, + }, + }, + { + name: "additional-trust-bundle", + dependencies: []asset.Asset{ + &agent.OptionalInstallConfig{ + Supplied: true, + InstallConfig: installconfig.InstallConfig{ + Config: &types.InstallConfig{ + ObjectMeta: v1.ObjectMeta{ + Namespace: "cluster-0", + }, + AdditionalTrustBundle: ` +-----BEGIN CERTIFICATE----- +MIIDZTCCAk2gAwIBAgIURbA8lR+5xlJZUoOXK66AHFWd3uswDQYJKoZIhvcNAQEL +BQAwQjELMAkGA1UEBhMCWFgxFTATBgNVBAcMDERlZmF1bHQgQ2l0eTEcMBoGA1UE +CgwTRGVmYXVsdCBDb21wYW55IEx0ZDAeFw0yMjA3MDgxOTUzMTVaFw0yMjA4MDcx +OTUzMTVaMEIxCzAJBgNVBAYTAlhYMRUwEwYDVQQHDAxEZWZhdWx0IENpdHkxHDAa +BgNVBAoME0RlZmF1bHQgQ29tcGFueSBMdGQwggEiMA0GCSqGSIb3DQEBAQUAA4IB +DwAwggEKAoIBAQCroH9c2PLWI0O/nBrmKtS2IuReyWaR0DOMJY7C/vc12l9zlH0D +xTOUfEtdqRktjVsUn1vIIiFakxd0QLIPcMyKplmbavIBUQp+MZr0pNVX+lwcctbA +7FVHEnbWYNVepoV7kZkTVvMXAqFylMXU4gDmuZzIxhVMMxjialJNED+3ngqvX4w3 +4q4KSk1ytaHGwjREIErwPJjv5PK48KVJL2nlCuA+tbxu1r8eVkOUvZlxAuNNXk/U +mf3QX5EiUlTtsmRAct6fIUT3jkrsHSS/tZ66EYJ9Q0OBoX2lL/Msmi27OQvA7uYn +uqYlwJzU43tCsiip9E9z/UrLcMYyXx3oPJyPAgMBAAGjUzBRMB0GA1UdDgQWBBTI +ahE8DDT4T1vta6cXVVaRjnel0zAfBgNVHSMEGDAWgBTIahE8DDT4T1vta6cXVVaR +jnel0zAPBgNVHRMBAf8EBTADAQH/MA0GCSqGSIb3DQEBCwUAA4IBAQCQbsMtPFkq +PxwOAIds3IoupuyIKmsF32ECEH/OlS+7Sj7MUJnGTQrwgjrsVS5sl8AmnGx4hPdL +VX98nEcKMNkph3Hkvh4EvgjSfmYGUXuJBcYU5jqNQrlrGv37rEf5FnvdHV1F3MG8 +A0Mj0TLtcTdtaJFoOrnQuD/k0/1d+cMiYGTSaT5XK/unARqGEMd4BlWPh5P3SflV +/Vy2hHlMpv7OcZ8yaAI3htENZLus+L5kjHWKu6dxlPHKu6ef5k64su2LTNE07Vr9 +S655uiFW5AX2wDVUcQEDCOiEn6SI9DTt5oQjWPMxPf+rEyfQ2f1QwVez7cyr6Qc5 +OIUk31HnM/Fj +-----END CERTIFICATE----- +`, + }, + }, + }, + }, + expectedConfig: `-----BEGIN CERTIFICATE----- +MIIDZTCCAk2gAwIBAgIURbA8lR+5xlJZUoOXK66AHFWd3uswDQYJKoZIhvcNAQEL +BQAwQjELMAkGA1UEBhMCWFgxFTATBgNVBAcMDERlZmF1bHQgQ2l0eTEcMBoGA1UE +CgwTRGVmYXVsdCBDb21wYW55IEx0ZDAeFw0yMjA3MDgxOTUzMTVaFw0yMjA4MDcx +OTUzMTVaMEIxCzAJBgNVBAYTAlhYMRUwEwYDVQQHDAxEZWZhdWx0IENpdHkxHDAa +BgNVBAoME0RlZmF1bHQgQ29tcGFueSBMdGQwggEiMA0GCSqGSIb3DQEBAQUAA4IB +DwAwggEKAoIBAQCroH9c2PLWI0O/nBrmKtS2IuReyWaR0DOMJY7C/vc12l9zlH0D +xTOUfEtdqRktjVsUn1vIIiFakxd0QLIPcMyKplmbavIBUQp+MZr0pNVX+lwcctbA +7FVHEnbWYNVepoV7kZkTVvMXAqFylMXU4gDmuZzIxhVMMxjialJNED+3ngqvX4w3 +4q4KSk1ytaHGwjREIErwPJjv5PK48KVJL2nlCuA+tbxu1r8eVkOUvZlxAuNNXk/U +mf3QX5EiUlTtsmRAct6fIUT3jkrsHSS/tZ66EYJ9Q0OBoX2lL/Msmi27OQvA7uYn +uqYlwJzU43tCsiip9E9z/UrLcMYyXx3oPJyPAgMBAAGjUzBRMB0GA1UdDgQWBBTI +ahE8DDT4T1vta6cXVVaRjnel0zAfBgNVHSMEGDAWgBTIahE8DDT4T1vta6cXVVaR +jnel0zAPBgNVHRMBAf8EBTADAQH/MA0GCSqGSIb3DQEBCwUAA4IBAQCQbsMtPFkq +PxwOAIds3IoupuyIKmsF32ECEH/OlS+7Sj7MUJnGTQrwgjrsVS5sl8AmnGx4hPdL +VX98nEcKMNkph3Hkvh4EvgjSfmYGUXuJBcYU5jqNQrlrGv37rEf5FnvdHV1F3MG8 +A0Mj0TLtcTdtaJFoOrnQuD/k0/1d+cMiYGTSaT5XK/unARqGEMd4BlWPh5P3SflV +/Vy2hHlMpv7OcZ8yaAI3htENZLus+L5kjHWKu6dxlPHKu6ef5k64su2LTNE07Vr9 +S655uiFW5AX2wDVUcQEDCOiEn6SI9DTt5oQjWPMxPf+rEyfQ2f1QwVez7cyr6Qc5 +OIUk31HnM/Fj +-----END CERTIFICATE----- +`, + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + + parents := asset.Parents{} + parents.Add(tc.dependencies...) + + asset := &CaBundle{} + err := asset.Generate(parents) + + if tc.expectedError != "" { + assert.EqualError(t, err, tc.expectedError) + } else { + assert.NoError(t, err) + + files := asset.Files() + if tc.expectedConfig != "" { + assert.Len(t, files, 1) + assert.Equal(t, CaBundleFilename, files[0].Filename) + assert.Equal(t, tc.expectedConfig, string(files[0].Data)) + } else { + assert.Empty(t, files) + } + } + }) + } +} + func TestCaBundle_LoadedFromDisk(t *testing.T) { cases := []struct { diff --git a/pkg/asset/manifests/additionaltrustbundleconfig.go b/pkg/asset/manifests/additionaltrustbundleconfig.go index c13482e59e6..ae11aa39569 100644 --- a/pkg/asset/manifests/additionaltrustbundleconfig.go +++ b/pkg/asset/manifests/additionaltrustbundleconfig.go @@ -56,7 +56,7 @@ func (atbc *AdditionalTrustBundleConfig) Generate(dependencies asset.Parents) er if installConfig.Config.AdditionalTrustBundle == "" { return nil } - data, err := parseCertificates(installConfig.Config.AdditionalTrustBundle) + data, err := ParseCertificates(installConfig.Config.AdditionalTrustBundle) if err != nil { return err @@ -99,7 +99,8 @@ func (atbc *AdditionalTrustBundleConfig) Load(f asset.FileFetcher) (bool, error) return false, nil } -func parseCertificates(certificates string) (map[string]string, error) { +// ParseCertificates parses and verifies a PEM certificate bundle +func ParseCertificates(certificates string) (map[string]string, error) { rest := []byte(certificates) var sb strings.Builder for { From a981f6df7fa1124bda9f6064945897218f4ba110 Mon Sep 17 00:00:00 2001 From: Zane Bitter Date: Fri, 8 Jul 2022 15:40:38 -0400 Subject: [PATCH 2/8] Validate ca-bundle.crt when loaded from disk --- pkg/asset/agent/mirror/cabundle.go | 14 ++++++++--- pkg/asset/agent/mirror/cabundle_test.go | 33 +++++++++++++++++++++++-- 2 files changed, 42 insertions(+), 5 deletions(-) diff --git a/pkg/asset/agent/mirror/cabundle.go b/pkg/asset/agent/mirror/cabundle.go index 1cd43c974a4..99d9881a9da 100644 --- a/pkg/asset/agent/mirror/cabundle.go +++ b/pkg/asset/agent/mirror/cabundle.go @@ -47,7 +47,16 @@ func (i *CaBundle) Generate(dependencies asset.Parents) error { if installConfig.Config.AdditionalTrustBundle == "" { return nil } - data, err := manifests.ParseCertificates(installConfig.Config.AdditionalTrustBundle) + + return i.parseCertificates(installConfig.Config.AdditionalTrustBundle) +} + +func (i *CaBundle) parseCertificates(certs string) error { + if len(certs) == 0 { + return nil + } + + data, err := manifests.ParseCertificates(certs) if err != nil { return err } @@ -85,6 +94,5 @@ func (i *CaBundle) Load(f asset.FileFetcher) (bool, error) { return false, errors.Wrap(err, fmt.Sprintf("failed to load %s file", CaBundleFilename)) } - i.File = file - return true, nil + return true, i.parseCertificates(string(file.Data)) } diff --git a/pkg/asset/agent/mirror/cabundle_test.go b/pkg/asset/agent/mirror/cabundle_test.go index 52479fc9d97..92c71e68c2f 100644 --- a/pkg/asset/agent/mirror/cabundle_test.go +++ b/pkg/asset/agent/mirror/cabundle_test.go @@ -146,11 +146,40 @@ func TestCaBundle_LoadedFromDisk(t *testing.T) { name: "valid-config-file", data: ` -----BEGIN CERTIFICATE----- -MIIDvTCCAqWgAwIBAgICEAA.... ------ END CERTIFICATE -----`, +MIIDZTCCAk2gAwIBAgIURbA8lR+5xlJZUoOXK66AHFWd3uswDQYJKoZIhvcNAQEL +BQAwQjELMAkGA1UEBhMCWFgxFTATBgNVBAcMDERlZmF1bHQgQ2l0eTEcMBoGA1UE +CgwTRGVmYXVsdCBDb21wYW55IEx0ZDAeFw0yMjA3MDgxOTUzMTVaFw0yMjA4MDcx +OTUzMTVaMEIxCzAJBgNVBAYTAlhYMRUwEwYDVQQHDAxEZWZhdWx0IENpdHkxHDAa +BgNVBAoME0RlZmF1bHQgQ29tcGFueSBMdGQwggEiMA0GCSqGSIb3DQEBAQUAA4IB +DwAwggEKAoIBAQCroH9c2PLWI0O/nBrmKtS2IuReyWaR0DOMJY7C/vc12l9zlH0D +xTOUfEtdqRktjVsUn1vIIiFakxd0QLIPcMyKplmbavIBUQp+MZr0pNVX+lwcctbA +7FVHEnbWYNVepoV7kZkTVvMXAqFylMXU4gDmuZzIxhVMMxjialJNED+3ngqvX4w3 +4q4KSk1ytaHGwjREIErwPJjv5PK48KVJL2nlCuA+tbxu1r8eVkOUvZlxAuNNXk/U +mf3QX5EiUlTtsmRAct6fIUT3jkrsHSS/tZ66EYJ9Q0OBoX2lL/Msmi27OQvA7uYn +uqYlwJzU43tCsiip9E9z/UrLcMYyXx3oPJyPAgMBAAGjUzBRMB0GA1UdDgQWBBTI +ahE8DDT4T1vta6cXVVaRjnel0zAfBgNVHSMEGDAWgBTIahE8DDT4T1vta6cXVVaR +jnel0zAPBgNVHRMBAf8EBTADAQH/MA0GCSqGSIb3DQEBCwUAA4IBAQCQbsMtPFkq +PxwOAIds3IoupuyIKmsF32ECEH/OlS+7Sj7MUJnGTQrwgjrsVS5sl8AmnGx4hPdL +VX98nEcKMNkph3Hkvh4EvgjSfmYGUXuJBcYU5jqNQrlrGv37rEf5FnvdHV1F3MG8 +A0Mj0TLtcTdtaJFoOrnQuD/k0/1d+cMiYGTSaT5XK/unARqGEMd4BlWPh5P3SflV +/Vy2hHlMpv7OcZ8yaAI3htENZLus+L5kjHWKu6dxlPHKu6ef5k64su2LTNE07Vr9 +S655uiFW5AX2wDVUcQEDCOiEn6SI9DTt5oQjWPMxPf+rEyfQ2f1QwVez7cyr6Qc5 +OIUk31HnM/Fj +-----END CERTIFICATE----- +`, expectedFound: true, expectedError: "", }, + { + name: "invalid-config-file", + data: ` +-----BEGIN CERTIFICATE----- +nope +-----END CERTIFICATE----- +`, + expectedFound: true, + expectedError: "x509: malformed certificate", + }, { name: "empty", data: "", From 04acaba54e6513071caf321d7a33024f7423fd0a Mon Sep 17 00:00:00 2001 From: Zane Bitter Date: Thu, 7 Jul 2022 13:59:31 -0400 Subject: [PATCH 3/8] Generate a default registries.conf file Use the podman default, with no unqualified-search-registries. --- pkg/asset/agent/mirror/registriesconf.go | 85 ++++++++++++++++++++++++ 1 file changed, 85 insertions(+) diff --git a/pkg/asset/agent/mirror/registriesconf.go b/pkg/asset/agent/mirror/registriesconf.go index 598cc8f5785..27faa21533d 100644 --- a/pkg/asset/agent/mirror/registriesconf.go +++ b/pkg/asset/agent/mirror/registriesconf.go @@ -16,6 +16,87 @@ var ( RegistriesConfFilename = filepath.Join(mirrorConfigDir, "registries.conf") ) +// The default registries.conf file is the podman default as it appears in +// CoreOS, with no unqualified-search-registries. +const defaultRegistriesConf = ` +# NOTE: RISK OF USING UNQUALIFIED IMAGE NAMES +# We recommend always using fully qualified image names including the registry +# server (full dns name), namespace, image name, and tag +# (e.g., registry.redhat.io/ubi8/ubi:latest). Pulling by digest (i.e., +# quay.io/repository/name@digest) further eliminates the ambiguity of tags. +# When using short names, there is always an inherent risk that the image being +# pulled could be spoofed. For example, a user wants to pull an image named +# 'foobar' from a registry and expects it to come from myregistry.com. If +# myregistry.com is not first in the search list, an attacker could place a +# different 'foobar' image at a registry earlier in the search list. The user +# would accidentally pull and run the attacker's image and code rather than the +# intended content. We recommend only adding registries which are completely +# trusted (i.e., registries which don't allow unknown or anonymous users to +# create accounts with arbitrary names). This will prevent an image from being +# spoofed, squatted or otherwise made insecure. If it is necessary to use one +# of these registries, it should be added at the end of the list. +# +# # An array of host[:port] registries to try when pulling an unqualified image, in order. + +unqualified-search-registries = [] + +# [[registry]] +# # The "prefix" field is used to choose the relevant [[registry]] TOML table; +# # (only) the TOML table with the longest match for the input image name +# # (taking into account namespace/repo/tag/digest separators) is used. +# # +# # The prefix can also be of the form: *.example.com for wildcard subdomain +# # matching. +# # +# # If the prefix field is missing, it defaults to be the same as the "location" field. +# prefix = "example.com/foo" +# +# # If true, unencrypted HTTP as well as TLS connections with untrusted +# # certificates are allowed. +# insecure = false +# +# # If true, pulling images with matching names is forbidden. +# blocked = false +# +# # The physical location of the "prefix"-rooted namespace. +# # +# # By default, this is equal to "prefix" (in which case "prefix" can be omitted +# # and the [[registry]] TOML table can only specify "location"). +# # +# # Example: Given +# # prefix = "example.com/foo" +# # location = "internal-registry-for-example.net/bar" +# # requests for the image example.com/foo/myimage:latest will actually work with the +# # internal-registry-for-example.net/bar/myimage:latest image. +# +# # The location can be empty iff prefix is in a +# # wildcarded format: "*.example.com". In this case, the input reference will +# # be used as-is without any rewrite. +# location = internal-registry-for-example.com/bar" +# +# # (Possibly-partial) mirrors for the "prefix"-rooted namespace. +# # +# # The mirrors are attempted in the specified order; the first one that can be +# # contacted and contains the image will be used (and if none of the mirrors contains the image, +# # the primary location specified by the "registry.location" field, or using the unmodified +# # user-specified reference, is tried last). +# # +# # Each TOML table in the "mirror" array can contain the following fields, with the same semantics +# # as if specified in the [[registry]] TOML table directly: +# # - location +# # - insecure +# [[registry.mirror]] +# location = "example-mirror-0.local/mirror-for-foo" +# [[registry.mirror]] +# location = "example-mirror-1.local/mirrors/foo" +# insecure = true +# # Given the above, a pull of example.com/foo/image:latest will try: +# # 1. example-mirror-0.local/mirror-for-foo/image:latest +# # 2. example-mirror-1.local/mirrors/foo/image:latest +# # 3. internal-registry-for-example.net/bar/image:latest +# # in order, and use the first one that exists. +` + // RegistriesConf generates the registries.conf file. type RegistriesConf struct { File *asset.File @@ -45,6 +126,10 @@ func (*RegistriesConf) Dependencies() []asset.Asset { // Generate generates the registries.conf file from install-config. func (i *RegistriesConf) Generate(dependencies asset.Parents) error { + i.File = &asset.File{ + Filename: RegistriesConfFilename, + Data: []byte(defaultRegistriesConf), + } // installConfig := &agent.OptionalInstallConfig{} From d823e9aef95fe310ebbefe46c9f881cb5a4e950b Mon Sep 17 00:00:00 2001 From: Zane Bitter Date: Tue, 5 Jul 2022 16:32:19 -0400 Subject: [PATCH 4/8] Generate mirror registries.conf from install-config if available --- pkg/asset/agent/mirror/registriesconf.go | 58 ++++++---- pkg/asset/agent/mirror/registriesconf_test.go | 105 ++++++++++++++++++ pkg/asset/ignition/bootstrap/common.go | 2 +- pkg/asset/ignition/bootstrap/registries.go | 4 +- .../ignition/bootstrap/registries_test.go | 2 +- 5 files changed, 144 insertions(+), 27 deletions(-) diff --git a/pkg/asset/agent/mirror/registriesconf.go b/pkg/asset/agent/mirror/registriesconf.go index 27faa21533d..10b4e329d50 100644 --- a/pkg/asset/agent/mirror/registriesconf.go +++ b/pkg/asset/agent/mirror/registriesconf.go @@ -5,8 +5,10 @@ import ( "os" "path/filepath" + "github.com/containers/image/pkg/sysregistriesv2" "github.com/openshift/installer/pkg/asset" "github.com/openshift/installer/pkg/asset/agent" + "github.com/openshift/installer/pkg/asset/ignition/bootstrap" "github.com/pelletier/go-toml" "github.com/pkg/errors" ) @@ -126,36 +128,44 @@ func (*RegistriesConf) Dependencies() []asset.Asset { // Generate generates the registries.conf file from install-config. func (i *RegistriesConf) Generate(dependencies asset.Parents) error { - i.File = &asset.File{ - Filename: RegistriesConfFilename, - Data: []byte(defaultRegistriesConf), + installConfig := &agent.OptionalInstallConfig{} + dependencies.Get(installConfig) + if !installConfig.Supplied || len(installConfig.Config.ImageContentSources) == 0 { + i.File = &asset.File{ + Filename: RegistriesConfFilename, + Data: []byte(defaultRegistriesConf), + } + return nil } - // installConfig := &agent.OptionalInstallConfig{} - - // registries := []sysregistriesv2.Registry{} - // for _, group := range bootstrap.MergedMirrorSets(installConfig.Config.ImageContentSources) { - // if len(group.Mirrors) == 0 { - // continue - // } + registries := sysregistriesv2.V2RegistriesConf{ + Registries: []sysregistriesv2.Registry{}, + } + for _, group := range bootstrap.MergedMirrorSets(installConfig.Config.ImageContentSources) { + if len(group.Mirrors) == 0 { + continue + } - // registry := sysregistriesv2.Registry{} - // registry.Endpoint.Location = group.Source - // registry.MirrorByDigestOnly = true - // for _, mirror := range group.Mirrors { - // registry.Mirrors = append(registry.Mirrors, sysregistriesv2.Endpoint{Location: mirror}) - // } - // registries = append(registries, registry) - // } + registry := sysregistriesv2.Registry{} + registry.Endpoint.Location = group.Source + registry.MirrorByDigestOnly = true + for _, mirror := range group.Mirrors { + registry.Mirrors = append(registry.Mirrors, sysregistriesv2.Endpoint{Location: mirror}) + } + registries.Registries = append(registries.Registries, registry) + } - // i.File = &asset.File{ - // Filename: RegistriesConfFilename, - // Data: registries, - // } + data, err := toml.Marshal(registries) + if err != nil { + return err + } - // return i.finish() + i.File = &asset.File{ + Filename: RegistriesConfFilename, + Data: data, + } - return nil + return i.finish() } // Files returns the files generated by the asset. diff --git a/pkg/asset/agent/mirror/registriesconf_test.go b/pkg/asset/agent/mirror/registriesconf_test.go index ddf4e871214..cc335cef8a8 100644 --- a/pkg/asset/agent/mirror/registriesconf_test.go +++ b/pkg/asset/agent/mirror/registriesconf_test.go @@ -7,11 +7,116 @@ import ( "github.com/golang/mock/gomock" "github.com/stretchr/testify/assert" + v1 "k8s.io/apimachinery/pkg/apis/meta/v1" "github.com/openshift/installer/pkg/asset" + "github.com/openshift/installer/pkg/asset/agent" + "github.com/openshift/installer/pkg/asset/installconfig" "github.com/openshift/installer/pkg/asset/mock" + "github.com/openshift/installer/pkg/types" ) +func TestRegistriesConf_Generate(t *testing.T) { + + cases := []struct { + name string + dependencies []asset.Asset + expectedError string + expectedConfig string + }{ + { + name: "missing-config", + dependencies: []asset.Asset{ + &agent.OptionalInstallConfig{}, + }, + expectedConfig: defaultRegistriesConf, + }, + { + name: "default", + dependencies: []asset.Asset{ + &agent.OptionalInstallConfig{ + Supplied: true, + InstallConfig: installconfig.InstallConfig{ + Config: &types.InstallConfig{ + ObjectMeta: v1.ObjectMeta{ + Namespace: "cluster-0", + }, + }, + }, + }, + }, + expectedConfig: defaultRegistriesConf, + }, + { + name: "image-content-sources", + dependencies: []asset.Asset{ + &agent.OptionalInstallConfig{ + Supplied: true, + InstallConfig: installconfig.InstallConfig{ + Config: &types.InstallConfig{ + ObjectMeta: v1.ObjectMeta{ + Namespace: "cluster-0", + }, + ImageContentSources: []types.ImageContentSource{ + { + Source: "registry.ci.openshift.org/ocp/release", + Mirrors: []string{ + "virthost.ostest.test.metalkube.org:5000/localimages/local-release-image", + }, + }, + { + Source: "quay.io/openshift-release-dev/ocp-v4.0-art-dev", + Mirrors: []string{ + "virthost.ostest.test.metalkube.org:5000/localimages/local-release-image", + }, + }, + }, + }, + }, + }, + }, + expectedConfig: `unqualified-search-registries = [] + +[[registry]] + location = "registry.ci.openshift.org/ocp/release" + mirror-by-digest-only = true + prefix = "" + + [[registry.mirror]] + location = "virthost.ostest.test.metalkube.org:5000/localimages/local-release-image" + +[[registry]] + location = "quay.io/openshift-release-dev/ocp-v4.0-art-dev" + mirror-by-digest-only = true + prefix = "" + + [[registry.mirror]] + location = "virthost.ostest.test.metalkube.org:5000/localimages/local-release-image" +`, + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + + parents := asset.Parents{} + parents.Add(tc.dependencies...) + + asset := &RegistriesConf{} + err := asset.Generate(parents) + + if tc.expectedError != "" { + assert.EqualError(t, err, tc.expectedError) + } else { + assert.NoError(t, err) + + files := asset.Files() + assert.Len(t, files, 1) + assert.Equal(t, tc.expectedConfig, string(files[0].Data)) + } + }) + } +} + func TestRegistries_LoadedFromDisk(t *testing.T) { cases := []struct { diff --git a/pkg/asset/ignition/bootstrap/common.go b/pkg/asset/ignition/bootstrap/common.go index e2b00277748..9c1ab1bf73a 100644 --- a/pkg/asset/ignition/bootstrap/common.go +++ b/pkg/asset/ignition/bootstrap/common.go @@ -240,7 +240,7 @@ func (a *Common) getTemplateData(dependencies asset.Parents, bootstrapInPlace bo } registries := []sysregistriesv2.Registry{} - for _, group := range mergedMirrorSets(installConfig.Config.ImageContentSources) { + for _, group := range MergedMirrorSets(installConfig.Config.ImageContentSources) { if len(group.Mirrors) == 0 { continue } diff --git a/pkg/asset/ignition/bootstrap/registries.go b/pkg/asset/ignition/bootstrap/registries.go index 99389b855e9..e1cc5e54c96 100644 --- a/pkg/asset/ignition/bootstrap/registries.go +++ b/pkg/asset/ignition/bootstrap/registries.go @@ -6,7 +6,9 @@ import ( "github.com/openshift/installer/pkg/types" ) -func mergedMirrorSets(sources []types.ImageContentSource) []types.ImageContentSource { +// MergedMirrorSets consolidates a list of ImageContentSources so that each +// source appears only once. +func MergedMirrorSets(sources []types.ImageContentSource) []types.ImageContentSource { sourceSet := make(map[string][]string) mirrorSet := make(map[string]sets.String) orderedSources := []string{} diff --git a/pkg/asset/ignition/bootstrap/registries_test.go b/pkg/asset/ignition/bootstrap/registries_test.go index 604fda314f9..623695173cd 100644 --- a/pkg/asset/ignition/bootstrap/registries_test.go +++ b/pkg/asset/ignition/bootstrap/registries_test.go @@ -120,7 +120,7 @@ func TestMergedMirrorSets(t *testing.T) { }} for _, test := range tests { t.Run(test.name, func(t *testing.T) { - assert.Equal(t, test.expected, mergedMirrorSets(test.input)) + assert.Equal(t, test.expected, MergedMirrorSets(test.input)) }) } } From 0a3243cb747f8834f8712147386629f5c778a3f8 Mon Sep 17 00:00:00 2001 From: Zane Bitter Date: Thu, 7 Jul 2022 11:11:13 -0400 Subject: [PATCH 5/8] Generate mirror configuration with cluster-manifests When the user runs the "agent create cluster-manifests" command, also generate the mirror configuration from the install-config if it is available. --- cmd/openshift-install/agent.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/cmd/openshift-install/agent.go b/cmd/openshift-install/agent.go index 61dabdce618..5e709cf74f0 100644 --- a/cmd/openshift-install/agent.go +++ b/cmd/openshift-install/agent.go @@ -7,6 +7,7 @@ import ( "github.com/openshift/installer/pkg/asset" "github.com/openshift/installer/pkg/asset/agent/image" "github.com/openshift/installer/pkg/asset/agent/manifests" + "github.com/openshift/installer/pkg/asset/agent/mirror" "github.com/openshift/installer/pkg/asset/kubeconfig" ) @@ -34,6 +35,8 @@ var ( }, assets: []asset.WritableAsset{ &manifests.AgentManifests{}, + &mirror.RegistriesConf{}, + &mirror.CaBundle{}, }, } From 15db1cbb0816e767a0ca98076613ced6d04ff6fd Mon Sep 17 00:00:00 2001 From: Zane Bitter Date: Thu, 7 Jul 2022 16:55:59 -0400 Subject: [PATCH 6/8] Make mirror config mounts less conditional Ideally we would just always mount the same /etc/containers/ config (including registries.conf) and CA certificates into the assisted-service container as we have in the live ISO. Both are pulling images from the same location (since the assisted-service image itself will come from the release payload). However, the assisted-service code fails if there are no registry mirrors configured in registries.conf, so continue to make just this part conditional on there being some mirroring settings. --- .../units/assisted-service.service.template | 2 +- pkg/asset/agent/image/ignition.go | 83 ++++++++----------- pkg/asset/agent/image/ignition_test.go | 8 +- 3 files changed, 40 insertions(+), 53 deletions(-) diff --git a/data/data/agent/systemd/units/assisted-service.service.template b/data/data/agent/systemd/units/assisted-service.service.template index 41b2c0c086d..32343d41d20 100644 --- a/data/data/agent/systemd/units/assisted-service.service.template +++ b/data/data/agent/systemd/units/assisted-service.service.template @@ -12,7 +12,7 @@ EnvironmentFile=/usr/local/share/assisted-service/agent-images.env Restart=on-failure TimeoutStopSec=300 ExecStartPre=/bin/rm -f %t/%n.ctr-id -ExecStart=/usr/bin/podman run --cidfile=%t/%n.ctr-id --cgroups=no-conmon --log-driver=journald --rm --pod-id-file=%t/assisted-service-pod.pod-id --sdnotify=conmon --replace -d --name=service -v /opt/agent/tls:/opt/agent/tls:z {{.MirrorRegistriesMount}} {{.CaBundleMount}} --env-file=/usr/local/share/assisted-service/assisted-service.env --env-file=/usr/local/share/assisted-service/images.env --env-file=/etc/assisted-service/node0 --env-file=/usr/local/share/assisted-service/agent-images.env $SERVICE_IMAGE +ExecStart=/usr/bin/podman run --cidfile=%t/%n.ctr-id --cgroups=no-conmon --log-driver=journald --rm --pod-id-file=%t/assisted-service-pod.pod-id --sdnotify=conmon --replace -d --name=service -v /opt/agent/tls:/opt/agent/tls:z {{ if .HaveMirrorConfig }}-v /etc/containers:/etc/containers{{ end }} -v /etc/pki/ca-trust:/etc/pki/ca-trust --env-file=/usr/local/share/assisted-service/assisted-service.env --env-file=/usr/local/share/assisted-service/images.env --env-file=/etc/assisted-service/node0 --env-file=/usr/local/share/assisted-service/agent-images.env $SERVICE_IMAGE ExecStop=/usr/bin/podman stop --ignore --cidfile=%t/%n.ctr-id ExecStopPost=/usr/bin/podman rm -f --ignore --cidfile=%t/%n.ctr-id Type=notify diff --git a/pkg/asset/agent/image/ignition.go b/pkg/asset/agent/image/ignition.go index 7c8dac2be06..d8bd2057ef0 100644 --- a/pkg/asset/agent/image/ignition.go +++ b/pkg/asset/agent/image/ignition.go @@ -43,18 +43,17 @@ type agentTemplateData struct { PullSecret string // PullSecretToken is token to use for authentication when AUTH_TYPE=rhsso // in assisted-service - PullSecretToken string - NodeZeroIP string - AssistedServiceHost string - APIVIP string - ControlPlaneAgents int - WorkerAgents int - ReleaseImages string - ReleaseImage string - ReleaseImageMirror string - MirrorRegistriesMount string - CaBundleMount string - InfraEnvID string + PullSecretToken string + NodeZeroIP string + AssistedServiceHost string + APIVIP string + ControlPlaneAgents int + WorkerAgents int + ReleaseImages string + ReleaseImage string + ReleaseImageMirror string + HaveMirrorConfig bool + InfraEnvID string } var ( @@ -131,18 +130,6 @@ func (a *Ignition) Generate(dependencies asset.Parents) error { agentMirror := &mirror.AgentMirror{} dependencies.Get(agentMirror) - // Mount files for assisted-service - mirrorRegistriesMount := "" - caBundleMount := "" - for _, file := range agentMirror.FileList { - if file.Filename == mirror.RegistriesConfFilename { - mirrorRegistriesMount = fmt.Sprintf("-v %s:/etc/containers/registries.conf:z", filepath.Join("/etc/assisted", file.Filename)) - } - if file.Filename == mirror.CaBundleFilename { - caBundleMount = fmt.Sprintf("-v %s:/etc/pki/ca-trust/extracted/pem/tls-ca-bundle.pem:z", filepath.Join("/etc/assisted", file.Filename)) - } - } - // Get the mirror for release image releaseImageMirror := "" source := strings.Split(agentManifests.ClusterImageSet.Spec.ReleaseImage, ":") @@ -162,8 +149,7 @@ func (a *Ignition) Generate(dependencies asset.Parents) error { releaseImageList, agentManifests.ClusterImageSet.Spec.ReleaseImage, releaseImageMirror, - mirrorRegistriesMount, - caBundleMount, + len(agentMirror.MirrorConfig) > 0, agentManifests.AgentClusterInstall, infraEnvID) @@ -216,8 +202,10 @@ func (a *Ignition) Generate(dependencies asset.Parents) error { return nil } -func getTemplateData(pullSecret string, nodeZeroIP string, releaseImageList string, releaseImage string, - releaseImageMirror string, mirrorRegistriesMount string, caBundleMount string, agentClusterInstall *hiveext.AgentClusterInstall, infraEnvID string) *agentTemplateData { +func getTemplateData(pullSecret, nodeZeroIP, releaseImageList, releaseImage, + releaseImageMirror string, haveMirrorConfig bool, + agentClusterInstall *hiveext.AgentClusterInstall, + infraEnvID string) *agentTemplateData { serviceBaseURL := url.URL{ Scheme: "http", Host: net.JoinHostPort(nodeZeroIP, "8090"), @@ -225,21 +213,20 @@ func getTemplateData(pullSecret string, nodeZeroIP string, releaseImageList stri } return &agentTemplateData{ - ServiceProtocol: serviceBaseURL.Scheme, - ServiceBaseURL: serviceBaseURL.String(), - PullSecret: pullSecret, - PullSecretToken: "", - NodeZeroIP: serviceBaseURL.Hostname(), - AssistedServiceHost: serviceBaseURL.Host, - APIVIP: agentClusterInstall.Spec.APIVIP, - ControlPlaneAgents: agentClusterInstall.Spec.ProvisionRequirements.ControlPlaneAgents, - WorkerAgents: agentClusterInstall.Spec.ProvisionRequirements.WorkerAgents, - ReleaseImages: releaseImageList, - ReleaseImage: releaseImage, - ReleaseImageMirror: releaseImageMirror, - MirrorRegistriesMount: mirrorRegistriesMount, - CaBundleMount: caBundleMount, - InfraEnvID: infraEnvID, + ServiceProtocol: serviceBaseURL.Scheme, + ServiceBaseURL: serviceBaseURL.String(), + PullSecret: pullSecret, + PullSecretToken: "", + NodeZeroIP: serviceBaseURL.Hostname(), + AssistedServiceHost: serviceBaseURL.Host, + APIVIP: agentClusterInstall.Spec.APIVIP, + ControlPlaneAgents: agentClusterInstall.Spec.ProvisionRequirements.ControlPlaneAgents, + WorkerAgents: agentClusterInstall.Spec.ProvisionRequirements.WorkerAgents, + ReleaseImages: releaseImageList, + ReleaseImage: releaseImage, + ReleaseImageMirror: releaseImageMirror, + HaveMirrorConfig: haveMirrorConfig, + InfraEnvID: infraEnvID, } } @@ -290,10 +277,12 @@ func addMirrorData(config *igntypes.Config, agentMirror *mirror.AgentMirror) { // add mirror files to ignition for _, file := range agentMirror.FileList { - // These are required for assisted-service to build the ICSP for openshift-install - mirrorFile := ignition.FileFromBytes(filepath.Join(mirrorPath, filepath.Base(file.Filename)), - "root", 0600, file.Data) - config.Storage.Files = append(config.Storage.Files, mirrorFile) + // This is required for assisted-service to build the ICSP for openshift-install + if file.Filename == mirror.RegistriesConfFilename { + mirrorFile := ignition.FileFromBytes("/etc/containers/registries.conf", + "root", 0600, file.Data) + config.Storage.Files = append(config.Storage.Files, mirrorFile) + } // This is required for the agent to run the podman commands to the mirror if file.Filename == mirror.CaBundleFilename { diff --git a/pkg/asset/agent/image/ignition_test.go b/pkg/asset/agent/image/ignition_test.go index 560c37c7095..8c759bd997b 100644 --- a/pkg/asset/agent/image/ignition_test.go +++ b/pkg/asset/agent/image/ignition_test.go @@ -48,13 +48,12 @@ func TestIgnition_getTemplateData(t *testing.T) { } releaseImage := "quay.io:443/openshift-release-dev/ocp-release:4.10.0-rc.1-x86_64" releaseImageMirror := "virthost.ostest.test.metalkube.org:5000/localimages/local-release-image" - mirrorRegistriesMount := "-v /etc/assisted/mirror/registries.conf:/etc/containers/registries.conf" - caBundleMount := "-v /etc/assisted/mirror/ca-bundle.crt:/etc/pki/ca-trust/extracted/pem/tls-ca-bundle.pem" infraEnvID := "random-infra-env-id" + hasMirrorConfig := true releaseImageList, err := releaseImageList(clusterImageSet.Spec.ReleaseImage, "x86_64") assert.NoError(t, err) - templateData := getTemplateData(pullSecret, nodeZeroIP, releaseImageList, releaseImage, releaseImageMirror, mirrorRegistriesMount, caBundleMount, agentClusterInstall, infraEnvID) + templateData := getTemplateData(pullSecret, nodeZeroIP, releaseImageList, releaseImage, releaseImageMirror, hasMirrorConfig, agentClusterInstall, infraEnvID) assert.Equal(t, "http", templateData.ServiceProtocol) assert.Equal(t, "http://"+nodeZeroIP+":8090/", templateData.ServiceBaseURL) assert.Equal(t, pullSecret, templateData.PullSecret) @@ -67,8 +66,7 @@ func TestIgnition_getTemplateData(t *testing.T) { assert.Equal(t, releaseImageList, templateData.ReleaseImages) assert.Equal(t, releaseImage, templateData.ReleaseImage) assert.Equal(t, releaseImageMirror, templateData.ReleaseImageMirror) - assert.Equal(t, mirrorRegistriesMount, templateData.MirrorRegistriesMount) - assert.Equal(t, caBundleMount, templateData.CaBundleMount) + assert.Equal(t, hasMirrorConfig, templateData.HasMirrorConfig) assert.Equal(t, infraEnvID, templateData.InfraEnvID) } From 90af4609aef23d44c5ff5b05fa53883495ee5157 Mon Sep 17 00:00:00 2001 From: Zane Bitter Date: Thu, 7 Jul 2022 16:48:48 -0400 Subject: [PATCH 7/8] Eliminate unneeded asset AgentMirror Unlike with 'cluster-manifests', we'll never need to read user-supplied files with unknown names from the 'mirror' directory, so there is no point having an extra asset to collect data from the two mirror config assets together. --- pkg/asset/agent/image/ignition.go | 48 +++++++++----------- pkg/asset/agent/image/ignition_test.go | 6 +-- pkg/asset/agent/mirror/mirror.go | 62 +------------------------- 3 files changed, 24 insertions(+), 92 deletions(-) diff --git a/pkg/asset/agent/image/ignition.go b/pkg/asset/agent/image/ignition.go index d8bd2057ef0..4df0a99f482 100644 --- a/pkg/asset/agent/image/ignition.go +++ b/pkg/asset/agent/image/ignition.go @@ -28,7 +28,6 @@ import ( const manifestPath = "/etc/assisted/manifests" const hostnamesPath = "/etc/assisted/hostnames" const nmConnectionsPath = "/etc/assisted/network" -const mirrorPath = "/etc/assisted/mirror" // Ignition is an asset that generates the agent installer ignition file. type Ignition struct { @@ -88,7 +87,8 @@ func (a *Ignition) Dependencies() []asset.Asset { &tls.AdminKubeConfigSignerCertKey{}, &tls.AdminKubeConfigClientCertKey{}, &agentconfig.Asset{}, - &mirror.AgentMirror{}, + &mirror.RegistriesConf{}, + &mirror.CaBundle{}, } } @@ -127,13 +127,14 @@ func (a *Ignition) Generate(dependencies asset.Parents) error { return err } - agentMirror := &mirror.AgentMirror{} - dependencies.Get(agentMirror) + registriesConfig := &mirror.RegistriesConf{} + registryCABundle := &mirror.CaBundle{} + dependencies.Get(registriesConfig, registryCABundle) // Get the mirror for release image releaseImageMirror := "" source := strings.Split(agentManifests.ClusterImageSet.Spec.ReleaseImage, ":") - for _, config := range agentMirror.MirrorConfig { + for _, config := range registriesConfig.MirrorConfig { if config.Location == source[0] { // include the tag with the build release image releaseImageMirror = fmt.Sprintf("%s:%s", config.Mirror, source[1]) @@ -149,7 +150,7 @@ func (a *Ignition) Generate(dependencies asset.Parents) error { releaseImageList, agentManifests.ClusterImageSet.Spec.ReleaseImage, releaseImageMirror, - len(agentMirror.MirrorConfig) > 0, + len(registriesConfig.MirrorConfig) > 0, agentManifests.AgentClusterInstall, infraEnvID) @@ -194,7 +195,7 @@ func (a *Ignition) Generate(dependencies asset.Parents) error { addTLSData(&config, dependencies) - addMirrorData(&config, agentMirror) + addMirrorData(&config, registriesConfig, registryCABundle) addHostConfig(&config, agentConfigAsset) @@ -273,29 +274,20 @@ func addTLSData(config *igntypes.Config, dependencies asset.Parents) { } } -func addMirrorData(config *igntypes.Config, agentMirror *mirror.AgentMirror) { +func addMirrorData(config *igntypes.Config, registriesConfig *mirror.RegistriesConf, registryCABundle *mirror.CaBundle) { - // add mirror files to ignition - for _, file := range agentMirror.FileList { - // This is required for assisted-service to build the ICSP for openshift-install - if file.Filename == mirror.RegistriesConfFilename { - mirrorFile := ignition.FileFromBytes("/etc/containers/registries.conf", - "root", 0600, file.Data) - config.Storage.Files = append(config.Storage.Files, mirrorFile) - } - - // This is required for the agent to run the podman commands to the mirror - if file.Filename == mirror.CaBundleFilename { - mirrorFile := ignition.FileFromBytes("/etc/pki/ca-trust/source/anchors/domain.crt", - "root", 0600, file.Data) - config.Storage.Files = append(config.Storage.Files, mirrorFile) - } - if file.Filename == mirror.RegistriesConfFilename { - registriesFile := ignition.FileFromBytes("/etc/containers/registries.conf", - "root", 0600, file.Data) - config.Storage.Files = append(config.Storage.Files, registriesFile) + // This is required for assisted-service to build the ICSP for openshift-install + if registriesConfig.File != nil { + registriesFile := ignition.FileFromBytes("/etc/containers/registries.conf", + "root", 0600, registriesConfig.File.Data) + config.Storage.Files = append(config.Storage.Files, registriesFile) + } - } + // This is required for the agent to run the podman commands to the mirror + if registryCABundle.File != nil { + caFile := ignition.FileFromBytes("/etc/pki/ca-trust/source/anchors/domain.crt", + "root", 0600, registryCABundle.File.Data) + config.Storage.Files = append(config.Storage.Files, caFile) } } diff --git a/pkg/asset/agent/image/ignition_test.go b/pkg/asset/agent/image/ignition_test.go index 8c759bd997b..97c7091a3f6 100644 --- a/pkg/asset/agent/image/ignition_test.go +++ b/pkg/asset/agent/image/ignition_test.go @@ -49,11 +49,11 @@ func TestIgnition_getTemplateData(t *testing.T) { releaseImage := "quay.io:443/openshift-release-dev/ocp-release:4.10.0-rc.1-x86_64" releaseImageMirror := "virthost.ostest.test.metalkube.org:5000/localimages/local-release-image" infraEnvID := "random-infra-env-id" - hasMirrorConfig := true + haveMirrorConfig := true releaseImageList, err := releaseImageList(clusterImageSet.Spec.ReleaseImage, "x86_64") assert.NoError(t, err) - templateData := getTemplateData(pullSecret, nodeZeroIP, releaseImageList, releaseImage, releaseImageMirror, hasMirrorConfig, agentClusterInstall, infraEnvID) + templateData := getTemplateData(pullSecret, nodeZeroIP, releaseImageList, releaseImage, releaseImageMirror, haveMirrorConfig, agentClusterInstall, infraEnvID) assert.Equal(t, "http", templateData.ServiceProtocol) assert.Equal(t, "http://"+nodeZeroIP+":8090/", templateData.ServiceBaseURL) assert.Equal(t, pullSecret, templateData.PullSecret) @@ -66,7 +66,7 @@ func TestIgnition_getTemplateData(t *testing.T) { assert.Equal(t, releaseImageList, templateData.ReleaseImages) assert.Equal(t, releaseImage, templateData.ReleaseImage) assert.Equal(t, releaseImageMirror, templateData.ReleaseImageMirror) - assert.Equal(t, hasMirrorConfig, templateData.HasMirrorConfig) + assert.Equal(t, haveMirrorConfig, templateData.HaveMirrorConfig) assert.Equal(t, infraEnvID, templateData.InfraEnvID) } diff --git a/pkg/asset/agent/mirror/mirror.go b/pkg/asset/agent/mirror/mirror.go index fefe6f98d14..4971171b840 100644 --- a/pkg/asset/agent/mirror/mirror.go +++ b/pkg/asset/agent/mirror/mirror.go @@ -1,63 +1,3 @@ package mirror -import ( - "github.com/openshift/installer/pkg/asset" -) - -const ( - mirrorConfigDir = "mirror" -) - -var ( - _ asset.WritableAsset = (*AgentMirror)(nil) -) - -// AgentMirror generates all the files required for disconnected installations. -type AgentMirror struct { - FileList []*asset.File - MirrorConfig []RegistriesConfig -} - -// Name returns a human friendly name. -func (m *AgentMirror) Name() string { - return "Agent Mirror Files" -} - -// Dependencies returns all of the dependencies directly needed the asset. -func (m *AgentMirror) Dependencies() []asset.Asset { - return []asset.Asset{ - &RegistriesConf{}, - &CaBundle{}, - } -} - -// Generate generates the respective mirror files. -func (m *AgentMirror) Generate(dependencies asset.Parents) error { - for _, a := range []asset.WritableAsset{ - &RegistriesConf{}, - &CaBundle{}, - } { - dependencies.Get(a) - m.FileList = append(m.FileList, a.Files()...) - - switch v := a.(type) { - case *RegistriesConf: - m.MirrorConfig = v.MirrorConfig - } - } - - asset.SortFiles(m.FileList) - - return nil -} - -// Files returns the files generated by the asset. -func (m *AgentMirror) Files() []*asset.File { - return m.FileList -} - -// Load returns the mirror files from disk. -func (m *AgentMirror) Load(f asset.FileFetcher) (bool, error) { - - return false, nil -} +const mirrorConfigDir = "mirror" From c5fb531389d7476a311a880c991aa4ca290aef8a Mon Sep 17 00:00:00 2001 From: Zane Bitter Date: Tue, 12 Jul 2022 17:14:54 -0400 Subject: [PATCH 8/8] Generate empty ca-bundle.crt file It's not always obvious to users what files they can provide, so generate an empty ca-bundle.crt file in the "agent create cluster-manifests" command if there is no additionalTrustBundle specified in the install-config. --- pkg/asset/agent/image/ignition.go | 2 +- pkg/asset/agent/mirror/cabundle.go | 4 ++++ pkg/asset/agent/mirror/cabundle_test.go | 7 ++++++- 3 files changed, 11 insertions(+), 2 deletions(-) diff --git a/pkg/asset/agent/image/ignition.go b/pkg/asset/agent/image/ignition.go index 4df0a99f482..c18a6c9024a 100644 --- a/pkg/asset/agent/image/ignition.go +++ b/pkg/asset/agent/image/ignition.go @@ -284,7 +284,7 @@ func addMirrorData(config *igntypes.Config, registriesConfig *mirror.RegistriesC } // This is required for the agent to run the podman commands to the mirror - if registryCABundle.File != nil { + if registryCABundle.File != nil && len(registryCABundle.File.Data) > 0 { caFile := ignition.FileFromBytes("/etc/pki/ca-trust/source/anchors/domain.crt", "root", 0600, registryCABundle.File.Data) config.Storage.Files = append(config.Storage.Files, caFile) diff --git a/pkg/asset/agent/mirror/cabundle.go b/pkg/asset/agent/mirror/cabundle.go index 99d9881a9da..1387da47d4a 100644 --- a/pkg/asset/agent/mirror/cabundle.go +++ b/pkg/asset/agent/mirror/cabundle.go @@ -45,6 +45,10 @@ func (i *CaBundle) Generate(dependencies asset.Parents) error { } if installConfig.Config.AdditionalTrustBundle == "" { + i.File = &asset.File{ + Filename: CaBundleFilename, + Data: []byte{}, + } return nil } diff --git a/pkg/asset/agent/mirror/cabundle_test.go b/pkg/asset/agent/mirror/cabundle_test.go index 92c71e68c2f..61b9d1ec0b7 100644 --- a/pkg/asset/agent/mirror/cabundle_test.go +++ b/pkg/asset/agent/mirror/cabundle_test.go @@ -126,7 +126,12 @@ OIUk31HnM/Fj assert.Equal(t, CaBundleFilename, files[0].Filename) assert.Equal(t, tc.expectedConfig, string(files[0].Data)) } else { - assert.Empty(t, files) + if len(files) == 1 { + assert.Equal(t, CaBundleFilename, files[0].Filename) + assert.Equal(t, []byte{}, files[0].Data) + } else { + assert.Empty(t, files) + } } } })