Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 9 additions & 8 deletions pkg/controller/common/iri_secret_merger.go
Comment thread
sadasu marked this conversation as resolved.
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package common

import (
"crypto/subtle"
"encoding/base64"
"encoding/json"
"errors"
Expand Down Expand Up @@ -53,7 +54,7 @@ func NewIRISecretMerger(
if err != nil {
return "", "", fmt.Errorf("could not get ControllerConfig: %w", err)
}
return extractIRICredentials(secret, cconfig)
return ExtractIRICredentials(secret, cconfig)
},
}
}
Expand All @@ -72,7 +73,7 @@ func NewIRISecretMergerFromObjects(
if !iri {
return "", "", errIRIDisabled
}
return extractIRICredentials(secret, cconfig)
return ExtractIRICredentials(secret, cconfig)
},
}
}
Expand All @@ -91,7 +92,7 @@ func (m *IRISecretMerger) Merge(pullSecretRaw []byte) ([]byte, error) {
if err != nil {
return nil, err
}
merged, changed, err := mergeIRIRegistryCredentialsIntoPullSecret(pullSecretRaw, password, baseDomain)
merged, changed, err := MergeIRIRegistryCredentialsIntoPullSecret(pullSecretRaw, password, baseDomain)
if err != nil {
return nil, err
}
Expand All @@ -101,9 +102,9 @@ func (m *IRISecretMerger) Merge(pullSecretRaw []byte) ([]byte, error) {
return merged, nil
}

// extractIRICredentials validates and extracts the password and baseDomain from
// ExtractIRICredentials validates and extracts the password and baseDomain from
// the IRI credentials secret and ControllerConfig.
func extractIRICredentials(secret *corev1.Secret, cconfig *mcfgv1.ControllerConfig) (password, baseDomain string, err error) {
func ExtractIRICredentials(secret *corev1.Secret, cconfig *mcfgv1.ControllerConfig) (password, baseDomain string, err error) {
if secret == nil {
return "", "", fmt.Errorf("IRI registry credentials secret must not be nil")
}
Expand All @@ -124,13 +125,13 @@ func extractIRICredentials(secret *corev1.Secret, cconfig *mcfgv1.ControllerConf
return string(pw), bd, nil
}

// mergeIRIRegistryCredentialsIntoPullSecret merges IRI registry authentication
// MergeIRIRegistryCredentialsIntoPullSecret merges IRI registry authentication
// credentials into a dockerconfigjson pull secret. It adds auth entries for
// api-int.<baseDomain>:<IRIRegistryPort> (all nodes) and
// localhost:<IRIRegistryPort> (masters, where the registry runs locally).
// Returns the merged bytes, a boolean indicating whether the pull secret was
// changed, and any error.
func mergeIRIRegistryCredentialsIntoPullSecret(pullSecretRaw []byte, password, baseDomain string) ([]byte, bool, error) {
func MergeIRIRegistryCredentialsIntoPullSecret(pullSecretRaw []byte, password, baseDomain string) ([]byte, bool, error) {
// The IRI registry is reachable via api-int on all nodes, and also via
// localhost on master nodes where it runs locally. registries.conf mirror
// rules on masters use localhost:22625, so credentials must be present for
Expand Down Expand Up @@ -175,5 +176,5 @@ func mergeIRIRegistryCredentialsIntoPullSecret(pullSecretRaw []byte, password, b
// matches expected.
func pullSecretHasAuth(auths map[string]interface{}, host, expected string) bool {
e, ok := auths[host].(map[string]interface{})
return ok && e["auth"] == expected
return ok && subtle.ConstantTimeCompare([]byte(e["auth"].(string)), []byte(expected)) == 1
}
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ var updateBackoff = wait.Backoff{
// Controller defines the InternalReleaseImage controller.
type Controller struct {
client mcfgclientset.Interface
kubeClient clientset.Interface
eventRecorder record.EventRecorder

syncHandler func(mcp string) error
Expand Down Expand Up @@ -101,6 +102,7 @@ func New(

ctrl := &Controller{
client: mcfgClient,
kubeClient: kubeClient,
eventRecorder: ctrlcommon.NamespacedEventRecorder(eventBroadcaster.NewRecorder(scheme.Scheme, corev1.EventSource{Component: "machineconfigcontroller-internalreleaseimagecontroller"})),
queue: workqueue.NewTypedRateLimitingQueueWithConfig(
workqueue.DefaultTypedControllerRateLimiter[string](),
Expand Down
29 changes: 24 additions & 5 deletions pkg/controller/template/template_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -158,9 +158,28 @@ func New(
}

func (ctrl *Controller) filterSecret(secret *corev1.Secret) {
if secret.Name == "pull-secret" || secret.Name == ctrlcommon.InternalReleaseImageAuthSecretName {
// Check if this is the IRI auth secret
if secret.Namespace == ctrlcommon.MCONamespace && secret.Name == ctrlcommon.InternalReleaseImageAuthSecretName {
ctrl.enqueueController()
klog.Infof("Re-syncing ControllerConfig due to secret %s change", secret.Name)
klog.Infof("Re-syncing ControllerConfig due to secret %s/%s change", secret.Namespace, secret.Name)
return
}

// Check if this is the configured global pull secret
cfg, err := ctrl.ccLister.Get(ctrlcommon.ControllerConfigName)
if err != nil {
// If we can't get the ControllerConfig, we can't determine if this secret
// is the pull secret, so skip the check. The controller will eventually
// sync when the ControllerConfig is available.
klog.V(4).Infof("Could not get ControllerConfig to check secret %s/%s: %v", secret.Namespace, secret.Name, err)
return
}

if cfg.Spec.PullSecret != nil &&
secret.Namespace == cfg.Spec.PullSecret.Namespace &&
secret.Name == cfg.Spec.PullSecret.Name {
ctrl.enqueueController()
klog.Infof("Re-syncing ControllerConfig due to secret %s/%s change", secret.Namespace, secret.Name)
}
}

Expand All @@ -170,15 +189,15 @@ func (ctrl *Controller) addSecret(obj interface{}) {
ctrl.deleteSecret(secret)
return
}
klog.V(4).Infof("Add Secret %v", secret)
klog.V(4).Infof("Add Secret %s/%s", secret.Namespace, secret.Name)
ctrl.filterSecret(secret)
}

func (ctrl *Controller) updateSecret(old, newObj interface{}) {
oldSecret := old.(*corev1.Secret)
newSecret := newObj.(*corev1.Secret)

klog.V(4).Infof("Update Secret %v", newSecret)
klog.V(4).Infof("Update Secret %s/%s", newSecret.Namespace, newSecret.Name)

// Only trigger resync if the secret data actually changed
// This prevents log spam from informer resyncs and watch reconnections
Expand All @@ -189,7 +208,7 @@ func (ctrl *Controller) updateSecret(old, newObj interface{}) {

func (ctrl *Controller) deleteSecret(obj interface{}) {
secret, ok := obj.(*corev1.Secret)
klog.V(4).Infof("Delete Secret %v", secret)
klog.V(4).Infof("Delete Secret %s/%s", secret.Namespace, secret.Name)

if !ok {
tombstone, ok := obj.(cache.DeletedFinalStateUnknown)
Expand Down
4 changes: 2 additions & 2 deletions pkg/controller/template/template_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -530,8 +530,8 @@ func TestKubeletAutoNodeSizingEnabled(t *testing.T) {
}
}

// TestMergesIRIRegistryCredentialsIntoPullSecret verifies that the template controller merges
// IRI registry credentials into the pull secret when rendering 00-master, so that
// TestMergesIRIRegistryCredentialsIntoPullSecret verifies that the template
// controller merges IRI registry credentials into the rendered pull secret so
// nodes can authenticate to the IRI registry without writing to the user-controlled
// global pull secret.
func TestMergesIRIRegistryCredentialsIntoPullSecret(t *testing.T) {
Expand Down
1 change: 1 addition & 0 deletions pkg/operator/operator.go
Original file line number Diff line number Diff line change
Expand Up @@ -290,6 +290,7 @@ func New(
apiserverInformer.Informer(),
moscInformer.Informer(),
networkPolicyInformer.Informer(),
iriInformer.Informer(),
}
for _, i := range informers {
i.AddEventHandler(optr.eventHandler())
Expand Down
13 changes: 13 additions & 0 deletions pkg/operator/sync.go
Original file line number Diff line number Diff line change
Expand Up @@ -2324,6 +2324,19 @@ func (optr *Operator) getImageRegistryPullSecrets() ([]byte, error) {
return nil, fmt.Errorf("failed to marshal the merged pull secrets: %w", err)
}

// Ensure nodes can authenticate to the internal release image (IRI)
// registry when pulling images during OS updates. The credentials
// added here become part of ControllerConfig.Spec.InternalRegistryPullSecret,
// which is the auth source the OS-update path (rpm-ostree/bootc) uses;
// that path is separate from the kubelet's pull secret, so IRI credentials
// must be supplied here in addition to the render-time merge. When IRI is
// not enabled on the cluster this merge makes no changes.
iriMerger := ctrlcommon.NewIRISecretMerger(optr.mcoSecretLister, optr.ccLister, optr.iriLister)
mergedPullSecrets, err = iriMerger.Merge(mergedPullSecrets)
if err != nil {
return nil, fmt.Errorf("failed to merge IRI registry credentials into image registry pull secrets: %w", err)
}

return mergedPullSecrets, nil
}

Expand Down
133 changes: 133 additions & 0 deletions pkg/operator/sync_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ package operator

import (
"context"
"encoding/base64"
"encoding/json"
"testing"

configv1 "github.com/openshift/api/config/v1"
Expand All @@ -14,6 +16,7 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/client-go/informers"
"k8s.io/client-go/kubernetes/fake"
corev1listers "k8s.io/client-go/listers/core/v1"
"k8s.io/client-go/tools/cache"
"k8s.io/client-go/tools/record"

Expand Down Expand Up @@ -308,6 +311,136 @@ func TestMachineOSBuilderSecretReconciliation(t *testing.T) {
}
}

// newNamespacedIndexer returns an indexer configured with the namespace index,
// as required by the namespaced core listers (Secrets, ServiceAccounts).
func newNamespacedIndexer(objs ...interface{}) cache.Indexer {
idx := cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{cache.NamespaceIndex: cache.MetaNamespaceIndexFunc})
for _, o := range objs {
idx.Add(o)
}
return idx
}

// TestGetImageRegistryPullSecretsIRIMerge verifies that getImageRegistryPullSecrets
// merges InternalReleaseImage (IRI) registry credentials into the assembled
// image-registry pull secret when IRI is in use, and leaves the secret untouched
// when IRI is absent. This blob feeds ControllerConfig.Spec.InternalRegistryPullSecret,
// which the daemon writes to /etc/mco/internal-registry-pull-secret.json for the
// OS-update image-pull path.
func TestGetImageRegistryPullSecretsIRIMerge(t *testing.T) {
const (
baseDomain = "example.com"
iriPassword = "s3cr3t"
)
expectedIRIAuth := base64.StdEncoding.EncodeToString([]byte(ctrlcommon.IRIRegistryUsername + ":" + iriPassword))
iriAPIIntHost := "api-int." + baseDomain + ":22625"
iriLocalHost := "localhost:22625"

// Common fixtures independent of whether IRI is enabled.
imageRegistryCO := &configv1.ClusterOperator{ObjectMeta: metav1.ObjectMeta{Name: "image-registry"}}
clusterDNS := &configv1.DNS{
ObjectMeta: metav1.ObjectMeta{Name: "cluster"},
Spec: configv1.DNSSpec{BaseDomain: baseDomain},
}
// machine-os-puller SA with no image pull secrets; the cluster pull secret
// alone keeps the assembled "auths" map non-empty so the merge path runs.
machineOSPullerSA := &corev1.ServiceAccount{
ObjectMeta: metav1.ObjectMeta{Name: "machine-os-puller", Namespace: ctrlcommon.MCONamespace},
}
populatedPullSecretContent := `{"auths":{"registry.example.com":{"auth":"` +
base64.StdEncoding.EncodeToString([]byte("user:pass")) + `"}}}`

// IRI-specific fixtures.
iriInstance := &mcfgv1.InternalReleaseImage{
ObjectMeta: metav1.ObjectMeta{Name: ctrlcommon.InternalReleaseImageInstanceName},
}
iriAuthSecret := &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{Name: ctrlcommon.InternalReleaseImageAuthSecretName, Namespace: ctrlcommon.MCONamespace},
Data: map[string][]byte{"password": []byte(iriPassword)},
}
controllerConfig := &mcfgv1.ControllerConfig{
ObjectMeta: metav1.ObjectMeta{Name: ctrlcommon.ControllerConfigName},
Spec: mcfgv1.ControllerConfigSpec{
DNS: &configv1.DNS{Spec: configv1.DNSSpec{BaseDomain: baseDomain}},
},
}

cases := []struct {
name string
// pullSecretContent is the raw ".dockerconfigjson" of the cluster pull secret.
pullSecretContent string
iriEnabled bool
// expectNil asserts getImageRegistryPullSecrets returns a nil secret,
// used for the empty-"auths" ("don't roll config") path.
expectNil bool
}{
{name: "IRI absent - pull secret unchanged", pullSecretContent: populatedPullSecretContent, iriEnabled: false},
{name: "IRI present - credentials merged", pullSecretContent: populatedPullSecretContent, iriEnabled: true},
// With no image-pull secrets on the SA and an empty cluster pull secret,
// the assembled "auths" map is empty. Even with IRI enabled the function
// must return nil rather than emitting a secret carrying only IRI creds.
{name: "empty auths with IRI enabled - returns nil", pullSecretContent: "{}", iriEnabled: true, expectNil: true},
}

for _, tc := range cases {
tc := tc
t.Run(tc.name, func(t *testing.T) {
t.Parallel()

mcoSecretObjs := []interface{}{}
iriObjs := []interface{}{}
if tc.iriEnabled {
mcoSecretObjs = append(mcoSecretObjs, iriAuthSecret)
iriObjs = append(iriObjs, iriInstance)
}

clusterPullSecret := helpers.NewDockerCfgJSONSecret(ctrlcommon.GlobalPullSecretName, ctrlcommon.OpenshiftConfigNamespace, tc.pullSecretContent)

optr := &Operator{
namespace: ctrlcommon.MCONamespace,
clusterOperatorLister: configlistersv1.NewClusterOperatorLister(newNamespacedIndexer(imageRegistryCO)),
dnsLister: configlistersv1.NewDNSLister(newNamespacedIndexer(clusterDNS)),
mcoSALister: corev1listers.NewServiceAccountLister(newNamespacedIndexer(machineOSPullerSA)),
mcoSecretLister: corev1listers.NewSecretLister(newNamespacedIndexer(mcoSecretObjs...)),
ocSecretLister: corev1listers.NewSecretLister(newNamespacedIndexer(clusterPullSecret)),
ccLister: mcplister.NewControllerConfigLister(newNamespacedIndexer(controllerConfig)),
iriLister: mcplister.NewInternalReleaseImageLister(newNamespacedIndexer(iriObjs...)),
}

raw, err := optr.getImageRegistryPullSecrets()
assert.NoError(t, err)

if tc.expectNil {
// Empty "auths": nothing to roll, so no secret is emitted (and
// IRI creds are not emitted on their own).
assert.Nil(t, raw)
return
}
assert.NotEmpty(t, raw)

var parsed struct {
Auths map[string]struct {
Auth string `json:"auth"`
} `json:"auths"`
}
assert.NoError(t, json.Unmarshal(raw, &parsed))

// The original registry entry is always present.
assert.Contains(t, parsed.Auths, "registry.example.com")

if tc.iriEnabled {
assert.Contains(t, parsed.Auths, iriAPIIntHost, "expected api-int IRI auth entry to be merged")
assert.Contains(t, parsed.Auths, iriLocalHost, "expected localhost IRI auth entry to be merged")
assert.Equal(t, expectedIRIAuth, parsed.Auths[iriAPIIntHost].Auth)
assert.Equal(t, expectedIRIAuth, parsed.Auths[iriLocalHost].Auth)
} else {
assert.NotContains(t, parsed.Auths, iriAPIIntHost, "IRI auth entry must not be present when IRI is absent")
assert.NotContains(t, parsed.Auths, iriLocalHost, "IRI auth entry must not be present when IRI is absent")
}
})
}
}

func TestSyncMachineConfiguration(t *testing.T) {
cases := []struct {
name string
Expand Down