Skip to content

Commit

Permalink
Use the new recert additional trust bundle options
Browse files Browse the repository at this point in the history
# Background / Context

Recert recently added ([1], [2]) some options that allow changing the
cluster's trust bundle (it's recommended you read the PRs for more
background about this).

# Issue / Requirement / Reason for change

The lifecycle-agent doesn't make use of the new options added to recert

# Solution / Feature Overview

Change the lifecycle-agent to use the new options added to recert

# Implementation Details

Multiple new fields have been added.

- `AdditionalTrustBundle` in `SeedReconfiguration`. This represents the
  trust bundle to be used for seed-reconfiguration. This contains the
  user-ca-bundle contents, the proxy configmap name, and the proxy configmap
  contents.

- `AdditionalTrustBundle` in `SeedClusterInfo`. This represents the
  state of the trust bundle in the seed cluster. This is simply booleans
  indicating the presence or lack there-of of the user-ca-bundle and the
  proxy configmap name (only if it actually has contents, a configmap
  with no contents is considered invalid OCP configuration). This is
  useful for when we want to verify that the seed is compatible with
  our desired `SeedReconfiguration`.

- `RecertConfig` will now use the new `CryptoDirs` and `CryptoFiles` fields
  to specify the directories and files that should be considered part of
  the cluster's crypto material. Along with the `ClusterCustomizationDirs` and
  `ClusterCustomizationFiles` fields that specify the directories and files
  involved in cluster customization. Since these no longer overlap when
  it comes to customizing the trust bundle, we must use these new fields
  instead of the old common `StaticDirs` and `StaticFiles` fields.

[1] rh-ecosystem-edge/recert#110
[2] rh-ecosystem-edge/recert#140
  • Loading branch information
omertuc committed May 15, 2024
1 parent ef30b53 commit 0d07c12
Show file tree
Hide file tree
Showing 11 changed files with 309 additions and 120 deletions.
14 changes: 14 additions & 0 deletions api/seedreconfig/seedreconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,8 @@ type SeedReconfiguration struct {
// upgraded cluster. In IBI, a fake install-config should be generated by
// the IBIO. This parameter is required when the proxy parameters are set.
InstallConfig string `json:"install_config,omitempty"`

AdditionalTrustBundle AdditionalTrustBundle `json:"additionalTrustBundle,omitempty"`
}

type KubeConfigCryptoRetention struct {
Expand Down Expand Up @@ -174,3 +176,15 @@ type Proxy struct {
// +optional
NoProxy string `json:"noProxy,omitempty"`
}

type AdditionalTrustBundle struct {
// The contents of the "user-ca-bundle" configmap in the "openshift-config" namepace
UserCaBundle string `json:"userCaBundle"`

// The Proxy CR trustedCA configmap nam
ProxyConfigmapName string `json:"proxyConfigmapName"`

// The contents of the ProxyConfigmapName configmap. Must equal
// UserCaBundle if ProxyConfigmapName is "user-ca-bundle"
ProxyConfigmapBundle string `json:"proxyConfigmapBundle"`
}
42 changes: 42 additions & 0 deletions controllers/prep_handlers.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,23 @@ func GetSeedImage(c client.Client, ctx context.Context, ibu *lcav1alpha1.ImageBa
return fmt.Errorf("checking seed image compatibility: %w", err)
}

HasUserCaBundle, ProxyConfigmapName, err := lcautils.GetClusterAdditionalTrustBundleState(ctx, c)
if err != nil {
return fmt.Errorf("failed to get cluster additional trust bundle state: %w", err)
}

//nolint:staticcheck //lint:ignore SA9003 // else branch intentionally left empty
if seedInfo.AdditionalTrustBundle != nil {
if err := checkSeedImageAdditionalTrustBundleCompatibility(*seedInfo.AdditionalTrustBundle, HasUserCaBundle, ProxyConfigmapName); err != nil {
return fmt.Errorf("checking seed image additional trust bundle compatibility: %w", err)
}
} else {
// For the sake of backwards compatibility, we allow older seed images
// that don't have information about the additional trust bundle. This
// means that upgrade will fail at the recert stage if there's a
// mismatch between the seed and the seed reconfiguration data.
}

return nil
}

Expand Down Expand Up @@ -196,6 +213,31 @@ func checkSeedImageProxyCompatibility(seedHasProxy, hasProxy bool) error {
return nil
}

// checkSeedImageAdditionalTrustBundleCompatibility checks for proxy
// configuration compatibility of the seed image vs the current cluster. If the
// seed image has a proxy and the cluster being upgraded doesn't, we cannot
// proceed as recert does not support proxy rename under those conditions.
// Similarly, we cannot proceed if the cluster being upgraded has a proxy but
// the seed image doesn't.
func checkSeedImageAdditionalTrustBundleCompatibility(seedAdditionalTrustBundle seedclusterinfo.AdditionalTrustBundle, hasUserCaBundle bool, proxyConfigmapName string) error {
if seedAdditionalTrustBundle.HasUserCaBundle && !hasUserCaBundle {
return fmt.Errorf("seed image has an %s/%s configmap but the cluster being upgraded does not, this combination is not supported",
common.OpenshiftConfigNamespace, common.ClusterAdditionalTrustBundleName)
}

if !seedAdditionalTrustBundle.HasUserCaBundle && hasUserCaBundle {
return fmt.Errorf("seed image does not have an %s/%s configmap but the cluster being upgraded does, this combination is not supported",
common.OpenshiftConfigNamespace, common.ClusterAdditionalTrustBundleName)
}

if seedAdditionalTrustBundle.ProxyConfigmapName != proxyConfigmapName {
return fmt.Errorf("seed image's Proxy trustedCA configmap name %q (oc get proxy -oyaml) mismatches cluster's name %q, this combination is not supported",
seedAdditionalTrustBundle.ProxyConfigmapName, proxyConfigmapName)
}

return nil
}

// validateSeedOcpVersion rejects upgrade request if seed image version is not higher than current cluster (target) OCP version
func (r *ImageBasedUpgradeReconciler) validateSeedOcpVersion(seedOcpVersion string) error {
// get target OCP version
Expand Down
130 changes: 77 additions & 53 deletions internal/clusterconfig/clusterconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import (
v1 "github.com/openshift/api/config/v1"
operatorv1alpha1 "github.com/openshift/api/operator/v1alpha1"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
Expand All @@ -33,16 +32,11 @@ import (
const (
manifestDir = "manifests"

proxyName = "cluster"

pullSecretName = "pull-secret"

idmsFileName = "image-digest-mirror-set.json"
icspsFileName = "image-content-source-policy-list.json"

caBundleCMName = "user-ca-bundle"
caBundleFileName = caBundleCMName + ".json"

// ssh authorized keys file created by mco from ssh machine configs
sshKeyFile = "/home/core/.ssh/authorized_keys.d/ignition"
)
Expand Down Expand Up @@ -84,9 +78,6 @@ func (r *UpgradeClusterConfigGather) FetchClusterConfig(ctx context.Context, ost
if err := r.fetchClusterInfo(ctx, clusterConfigPath); err != nil {
return err
}
if err := r.fetchCABundle(ctx, manifestsDir, clusterConfigPath); err != nil {
return err
}
if err := r.fetchICSPs(ctx, manifestsDir); err != nil {
return err
}
Expand Down Expand Up @@ -144,9 +135,73 @@ func (r *UpgradeClusterConfigGather) GetKubeadminPasswordHash(ctx context.Contex
return kubeadminPasswordHash, nil
}

type AdditionalTrustBundle struct {
// The contents of the "user-ca-bundle" configmap in the "openshift-config" namepace
UserCaBundle string `json:"userCaBundle"`

// The Proxy CR trustedCA configmap name
ProxyConfigmapName string `json:"proxyConfigmapName"`

// The contents of the ProxyConfigmapName configmap. Must equal
// UserCaBundle if ProxyConfigmapName is "user-ca-bundle"
ProxyConfigmapBundle string `json:"proxyConfigmapBundle"`
}

func newAdditionalTrustBundle(userCaBundle, proxyConfigmapName, proxyConfigmapBundle string) (*AdditionalTrustBundle, error) {
if proxyConfigmapName == common.ClusterAdditionalTrustBundleName && userCaBundle != proxyConfigmapBundle {
return nil, fmt.Errorf("proxyConfigmapName is %s, but userCaBundle and proxyConfigmapBundle differ", common.ClusterAdditionalTrustBundleName)
}

return &AdditionalTrustBundle{
UserCaBundle: userCaBundle,
ProxyConfigmapName: proxyConfigmapName,
ProxyConfigmapBundle: proxyConfigmapBundle,
}, nil
}

func (r *UpgradeClusterConfigGather) GetAdditionalTrustBundle(ctx context.Context) (*AdditionalTrustBundle, error) {
clusterAdditionalTrustBundle, err := utils.GetAdditionalTrustBundleFromConfigmap(ctx, r.Client, common.ClusterAdditionalTrustBundleName)
if err != nil {
return nil, fmt.Errorf("failed to get additional trust bundle from configmap: %w", err)
}

proxy := v1.Proxy{}
if err := r.Client.Get(ctx, types.NamespacedName{Name: common.OpenshiftProxyCRName}, &proxy); err != nil {
return nil, fmt.Errorf("failed to get proxy: %w", err)

}

proxyCaBundle := ""
switch proxy.Spec.TrustedCA.Name {
case "":
// No proxy CA bundle
case common.ClusterAdditionalTrustBundleName:
// Proxy is using the cluster's CA bundle
proxyCaBundle = clusterAdditionalTrustBundle
default:
// Proxy is using a different CA bundle, retrieve it from the named configmap
proxyCaBundle, err = utils.GetAdditionalTrustBundleFromConfigmap(ctx, r.Client, proxy.Spec.TrustedCA.Name)
if err != nil {
return nil, fmt.Errorf("failed to get additional trust bundle from configmap: %w", err)
}

if proxyCaBundle == "" {
// This is a very weird but probably valid OCP configuration that we prefer to not support in LCA
return nil, fmt.Errorf("proxy trustedCA configmap %s/%s exists but is empty", common.OpenshiftConfigNamespace, proxy.Spec.TrustedCA.Name)
}
}

additionalTrustBundle, err := newAdditionalTrustBundle(clusterAdditionalTrustBundle, proxy.Spec.TrustedCA.Name, proxyCaBundle)
if err != nil {
return nil, fmt.Errorf("failed to create additional trust bundle: %w", err)
}

return additionalTrustBundle, err
}

func (r *UpgradeClusterConfigGather) GetProxy(ctx context.Context) (*seedreconfig.Proxy, *seedreconfig.Proxy, error) {
proxy := v1.Proxy{}
if err := r.Client.Get(ctx, types.NamespacedName{Name: proxyName}, &proxy); err != nil {
if err := r.Client.Get(ctx, types.NamespacedName{Name: common.OpenshiftProxyCRName}, &proxy); err != nil {
return nil, nil, fmt.Errorf("failed to get proxy: %w", err)
}

Expand Down Expand Up @@ -187,6 +242,7 @@ func SeedReconfigurationFromClusterInfo(
proxy,
statusProxy *seedreconfig.Proxy,
installConfig string,
additionalTrustBundle *AdditionalTrustBundle,
) *seedreconfig.SeedReconfiguration {
return &seedreconfig.SeedReconfiguration{
APIVersion: seedreconfig.SeedReconfigurationVersion,
Expand All @@ -204,6 +260,11 @@ func SeedReconfigurationFromClusterInfo(
Proxy: proxy,
StatusProxy: statusProxy,
InstallConfig: installConfig,
AdditionalTrustBundle: seedreconfig.AdditionalTrustBundle{
UserCaBundle: additionalTrustBundle.UserCaBundle,
ProxyConfigmapName: additionalTrustBundle.ProxyConfigmapName,
ProxyConfigmapBundle: additionalTrustBundle.ProxyConfigmapBundle,
},
}
}

Expand Down Expand Up @@ -245,6 +306,11 @@ func (r *UpgradeClusterConfigGather) fetchClusterInfo(ctx context.Context, clust
return err
}

additionalTrustBundle, err := r.GetAdditionalTrustBundle(ctx)
if err != nil {
return err
}

installConfig, err := r.GetInstallConfig(ctx)
if err != nil {
return err
Expand All @@ -258,6 +324,7 @@ func (r *UpgradeClusterConfigGather) fetchClusterInfo(ctx context.Context, clust
proxy,
statusProxy,
installConfig,
additionalTrustBundle,
)

filePath := filepath.Join(clusterConfigPath, common.SeedReconfigurationFileName)
Expand Down Expand Up @@ -317,14 +384,6 @@ func (r *UpgradeClusterConfigGather) typeMetaForObject(o runtime.Object) (*metav
}, nil
}

func (r *UpgradeClusterConfigGather) cleanObjectMetadata(o client.Object) metav1.ObjectMeta {
return metav1.ObjectMeta{
Name: o.GetName(),
Namespace: o.GetNamespace(),
Labels: o.GetLabels(),
}
}

func (r *UpgradeClusterConfigGather) getIDMSs(ctx context.Context) (v1.ImageDigestMirrorSetList, error) {
idmsList := v1.ImageDigestMirrorSetList{}
currentIdms := v1.ImageDigestMirrorSetList{}
Expand Down Expand Up @@ -400,41 +459,6 @@ func (r *UpgradeClusterConfigGather) fetchICSPs(ctx context.Context, manifestsDi
return nil
}

func (r *UpgradeClusterConfigGather) fetchCABundle(ctx context.Context, manifestsDir, clusterConfigPath string) error {
r.Log.Info("Fetching user ca bundle")
caBundle := &corev1.ConfigMap{}
err := r.Client.Get(ctx, types.NamespacedName{Name: caBundleCMName,
Namespace: common.OpenshiftConfigNamespace}, caBundle)
if err != nil && errors.IsNotFound(err) {
return nil
}
if err != nil {
return fmt.Errorf("failed to get ca bundle cm, err: %w", err)
}

typeMeta, err := r.typeMetaForObject(caBundle)
if err != nil {
return err
}
caBundle.TypeMeta = *typeMeta
caBundle.ObjectMeta = r.cleanObjectMetadata(caBundle)

if err := utils.MarshalToFile(caBundle, filepath.Join(manifestsDir, caBundleFileName)); err != nil {
return fmt.Errorf("failed to write user ca bundle to %s, err: %w",
filepath.Join(manifestsDir, caBundleFileName), err)
}

// we should copy ca-bundle from snoa as without doing it we will fail to pull images
// workaround for https://issues.redhat.com/browse/OCPBUGS-24035
caBundleFilePath := filepath.Join(hostPath, common.CABundleFilePath)
r.Log.Info("Copying", "file", caBundleFilePath)
if err := utils.CopyFileIfExists(caBundleFilePath, filepath.Join(clusterConfigPath, filepath.Base(caBundleFilePath))); err != nil {
return fmt.Errorf("failed to copy ca-bundle file %s to %s, err %w", caBundleFilePath, clusterConfigPath, err)
}

return nil
}

// gather network files and copy them
func (r *UpgradeClusterConfigGather) fetchNetworkConfig(ostreeDir string) error {
r.Log.Info("Fetching node network files")
Expand Down
26 changes: 1 addition & 25 deletions internal/clusterconfig/clusterconfig_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -393,7 +393,7 @@ func TestClusterConfig(t *testing.T) {
Name: "2",
}, Spec: operatorv1alpha1.ImageContentSourcePolicySpec{
RepositoryDigestMirrors: []operatorv1alpha1.RepositoryDigestMirrors{{Source: "icspData2"}}}}},
caBundleCM: &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Name: caBundleCMName,
caBundleCM: &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Name: common.ClusterAdditionalTrustBundleName,
Namespace: common.OpenshiftConfigNamespace}, Data: map[string]string{"test": "data"}},
expectedErr: false,
validateFunc: func(t *testing.T, tempDir string, err error, ucc UpgradeClusterConfigGather) {
Expand Down Expand Up @@ -427,17 +427,6 @@ func TestClusterConfig(t *testing.T) {
assert.Contains(t, resultSourcesAsString, "icspData")
assert.Contains(t, resultSourcesAsString, "icspData2")
}

// validate caBundle
caBundle := &corev1.ConfigMap{}
if err := utils.ReadYamlOrJSONFile(filepath.Join(manifestsDir, caBundleFileName), caBundle); err != nil {
t.Errorf("unexpected error: %v", err)
}
assert.Equal(t, caBundleCMName, caBundle.Name)
assert.Equal(t, caBundle.Data, map[string]string{"test": "data"})

_, err = os.Stat(filepath.Join(clusterConfigPath, filepath.Base(common.CABundleFilePath)))
assert.Nil(t, err)
},
},
}
Expand All @@ -447,19 +436,6 @@ func TestClusterConfig(t *testing.T) {
t.Run(testCase.testCaseName, func(t *testing.T) {
hostPath = clusterConfigDir

if testCase.caBundleCM != nil {
dir := filepath.Join(clusterConfigDir, filepath.Dir(common.CABundleFilePath))
if err := os.MkdirAll(dir, 0o700); err != nil {
t.Errorf("unexpected error: %v", err)
}
newPath := filepath.Join(dir, filepath.Base(common.CABundleFilePath))
f, err := os.Create(newPath)
if err != nil {
t.Errorf("unexpected error: %v", err)
}
_ = f.Close()
}

sshKeyDir := filepath.Join(clusterConfigDir, filepath.Dir(sshKeyFile))
if err := os.MkdirAll(sshKeyDir, 0o700); err != nil {
t.Errorf("unexpected error: %v", err)
Expand Down
4 changes: 3 additions & 1 deletion internal/common/consts.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@ const (
EtcdContainerName = "recert_etcd"
LvmConfigDir = "lvm-configuration"
LvmDevicesPath = "/etc/lvm/devices/system.devices"
CABundleFilePath = "/etc/pki/ca-trust/extracted/pem/tls-ca-bundle.pem"

LCAConfigDir = "/var/lib/lca"
IBUAutoRollbackConfigFile = LCAConfigDir + "/autorollback_config.json"
Expand Down Expand Up @@ -110,6 +109,9 @@ const (

NMConnectionFolder = "/etc/NetworkManager/system-connections"
NetworkDir = "network-configuration"

CaBundleDataKey = "ca-bundle.crt"
ClusterAdditionalTrustBundleName = "user-ca-bundle"
)

// Annotation names and values related to extra manifest
Expand Down
Loading

0 comments on commit 0d07c12

Please sign in to comment.