diff --git a/install/0000_80_machine-config_00_rbac.yaml b/install/0000_80_machine-config_00_rbac.yaml index 225de6fbb7..50f2fbaff7 100644 --- a/install/0000_80_machine-config_00_rbac.yaml +++ b/install/0000_80_machine-config_00_rbac.yaml @@ -144,6 +144,38 @@ subjects: roleRef: kind: Role name: host-networking-services + +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRole +metadata: + name: node-credential-providers + annotations: + include.release.openshift.io/self-managed-high-availability: "true" + include.release.openshift.io/single-node-developer: "true" +rules: + - apiGroups: [""] + resources: ["serviceaccounts"] + verbs: ["get", "list"] + - apiGroups: [""] + resources: ["https://kubernetes.default.svc"] + verbs: ["request-serviceaccounts-token-audience"] +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRoleBinding +metadata: + name: node-credential-providers-binding + annotations: + include.release.openshift.io/self-managed-high-availability: "true" + include.release.openshift.io/single-node-developer: "true" +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: ClusterRole + name: node-credential-providers +subjects: + - kind: Group + apiGroup: rbac.authorization.k8s.io + name: system:nodes --- apiVersion: rbac.authorization.k8s.io/v1 kind: RoleBinding diff --git a/manifests/machineconfigcontroller/clusterrole.yaml b/manifests/machineconfigcontroller/clusterrole.yaml index 64398641db..b06fd4bda5 100644 --- a/manifests/machineconfigcontroller/clusterrole.yaml +++ b/manifests/machineconfigcontroller/clusterrole.yaml @@ -13,10 +13,10 @@ rules: resources: ["configmaps", "secrets"] verbs: ["*"] - apiGroups: ["config.openshift.io"] - resources: ["images", "clusterversions", "featuregates", "nodes", "nodes/status", "imagepolicies/status"] + resources: ["images", "clusterversions", "featuregates", "nodes", "nodes/status", "imagepolicies/status", "criocredentialproviderconfigs/status"] verbs: ["*"] - apiGroups: ["config.openshift.io"] - resources: ["schedulers", "apiservers", "infrastructures", "imagedigestmirrorsets", "imagetagmirrorsets", "clusterimagepolicies", "imagepolicies"] + resources: ["schedulers", "apiservers", "infrastructures", "imagedigestmirrorsets", "imagetagmirrorsets", "clusterimagepolicies", "imagepolicies", "criocredentialproviderconfigs"] verbs: ["get", "list", "watch"] - apiGroups: ["operator.openshift.io"] resources: ["imagecontentsourcepolicies"] diff --git a/pkg/apihelpers/apihelpers.go b/pkg/apihelpers/apihelpers.go index c787365c93..64c672a487 100644 --- a/pkg/apihelpers/apihelpers.go +++ b/pkg/apihelpers/apihelpers.go @@ -108,6 +108,32 @@ var ( }, }, }, + // Add default policy for KubernetesCredentialProvidersDir + { + Path: constants.KubernetesCredentialProvidersDir, + Actions: []opv1.NodeDisruptionPolicyStatusAction{ + { + Type: opv1.RestartStatusAction, + Restart: &opv1.RestartService{ + ServiceName: "kubelet.service", + }, + }, + }, + }, + { + Path: constants.KubeletCrioImageCredProviderConfPath, + Actions: []opv1.NodeDisruptionPolicyStatusAction{ + { + Type: opv1.DaemonReloadStatusAction, + }, + { + Type: opv1.RestartStatusAction, + Restart: &opv1.RestartService{ + ServiceName: "kubelet.service", + }, + }, + }, + }, }, SSHKey: opv1.NodeDisruptionPolicyStatusSSHKey{ Actions: []opv1.NodeDisruptionPolicyStatusAction{ diff --git a/pkg/controller/container-runtime-config/container_runtime_config_controller.go b/pkg/controller/container-runtime-config/container_runtime_config_controller.go index 390663c721..b12f2b0800 100644 --- a/pkg/controller/container-runtime-config/container_runtime_config_controller.go +++ b/pkg/controller/container-runtime-config/container_runtime_config_controller.go @@ -3,6 +3,7 @@ package containerruntimeconfig import ( "context" "fmt" + "path/filepath" "reflect" "strconv" "strings" @@ -12,12 +13,14 @@ import ( signature "github.com/containers/image/v5/signature" ign3types "github.com/coreos/ignition/v2/config/v3_5/types" apicfgv1 "github.com/openshift/api/config/v1" + apicfgv1alpha1 "github.com/openshift/api/config/v1alpha1" features "github.com/openshift/api/features" apioperatorsv1alpha1 "github.com/openshift/api/operator/v1alpha1" configclientset "github.com/openshift/client-go/config/clientset/versioned" configinformers "github.com/openshift/client-go/config/informers/externalversions" cligoinformersv1 "github.com/openshift/client-go/config/informers/externalversions/config/v1" cligolistersv1 "github.com/openshift/client-go/config/listers/config/v1" + cligolistersv1alpha1 "github.com/openshift/client-go/config/listers/config/v1alpha1" runtimeutils "github.com/openshift/runtime-utils/pkg/registries" operatorinformersv1alpha1 "github.com/openshift/client-go/operator/informers/externalversions/operator/v1alpha1" @@ -26,6 +29,7 @@ import ( corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/equality" "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/types" @@ -49,6 +53,7 @@ import ( apihelpers "github.com/openshift/machine-config-operator/pkg/apihelpers" ctrlcommon "github.com/openshift/machine-config-operator/pkg/controller/common" mtmpl "github.com/openshift/machine-config-operator/pkg/controller/template" + "github.com/openshift/machine-config-operator/pkg/daemon/constants" "github.com/openshift/machine-config-operator/pkg/version" ) @@ -60,9 +65,10 @@ const ( // 5ms, 10ms, 20ms, 40ms, 80ms, 160ms, 320ms, 640ms, 1.3s, 2.6s, 5.1s, 10.2s, 20.4s, 41s, 82s maxRetries = 15 - builtInLabelKey = "machineconfiguration.openshift.io/mco-built-in" - configMapName = "crio-default-container-runtime" - forceSyncOnUpgrade = "force-sync-on-upgrade" + builtInLabelKey = "machineconfiguration.openshift.io/mco-built-in" + configMapName = "crio-default-container-runtime" + forceSyncOnUpgrade = "force-sync-on-upgrade" + genericCredProviderConfigPath = "/etc/kubernetes/credential-providers/generic-credential-provider.yaml" ) var ( @@ -87,6 +93,7 @@ type Controller struct { syncHandler func(mcp string) error syncImgHandler func(mcp string) error + syncCRIOCPHandler func(key string) error enqueueContainerRuntimeConfig func(*mcfgv1.ContainerRuntimeConfig) ccLister mcfglistersv1.ControllerConfigLister @@ -107,6 +114,10 @@ type Controller struct { itmsLister cligolistersv1.ImageTagMirrorSetLister itmsListerSynced cache.InformerSynced + criocpLister cligolistersv1alpha1.CRIOCredentialProviderConfigLister + criocpListerSynced cache.InformerSynced + addedCRIOCPObservers bool + configInformerFactory configinformers.SharedInformerFactory clusterImagePolicyLister cligolistersv1.ClusterImagePolicyLister clusterImagePolicyListerSynced cache.InformerSynced @@ -123,8 +134,9 @@ type Controller struct { fgHandler ctrlcommon.FeatureGatesHandler - queue workqueue.TypedRateLimitingInterface[string] - imgQueue workqueue.TypedRateLimitingInterface[string] + queue workqueue.TypedRateLimitingInterface[string] + imgQueue workqueue.TypedRateLimitingInterface[string] + criocpQueue workqueue.TypedRateLimitingInterface[string] } // New returns a new container runtime config controller @@ -157,7 +169,8 @@ func New( queue: workqueue.NewTypedRateLimitingQueueWithConfig( workqueue.DefaultTypedControllerRateLimiter[string](), workqueue.TypedRateLimitingQueueConfig[string]{Name: "machineconfigcontroller-containerruntimeconfigcontroller"}), - imgQueue: workqueue.NewTypedRateLimitingQueue(workqueue.DefaultTypedControllerRateLimiter[string]()), + imgQueue: workqueue.NewTypedRateLimitingQueue(workqueue.DefaultTypedControllerRateLimiter[string]()), + criocpQueue: workqueue.NewTypedRateLimitingQueue(workqueue.DefaultTypedControllerRateLimiter[string]()), } mcrInformer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{ @@ -192,6 +205,7 @@ func New( ctrl.syncHandler = ctrl.syncContainerRuntimeConfig ctrl.syncImgHandler = ctrl.syncImageConfig + ctrl.syncCRIOCPHandler = ctrl.syncCRIOCredentialProviderConfig ctrl.enqueueContainerRuntimeConfig = ctrl.enqueue ctrl.mcpLister = mcpInformer.Lister() @@ -231,15 +245,30 @@ func (ctrl *Controller) Run(workers int, stopCh <-chan struct{}) { defer utilruntime.HandleCrash() defer ctrl.queue.ShutDown() defer ctrl.imgQueue.ShutDown() + defer ctrl.criocpQueue.ShutDown() listerCaches := []cache.InformerSynced{ctrl.mcpListerSynced, ctrl.mccrListerSynced, ctrl.ccListerSynced, ctrl.imgListerSynced, ctrl.icspListerSynced, ctrl.idmsListerSynced, ctrl.itmsListerSynced, ctrl.clusterVersionListerSynced} if ctrl.sigstoreAPIEnabled() { ctrl.addImagePolicyObservers() klog.Info("addded image policy observers with sigstore featuregate enabled") + } + + if ctrl.criocpEnabled() { + ctrl.addCRIOCPObservers() + klog.Info("added CRIOCredentialProviderConfig observers with CRIOCredentialProviderConfig featuregate enabled") + } + + if ctrl.addedPolicyObservers || ctrl.addedCRIOCPObservers { ctrl.configInformerFactory.Start(stopCh) + } + + if ctrl.addedPolicyObservers { listerCaches = append(listerCaches, ctrl.clusterImagePolicyListerSynced, ctrl.imagePolicyListerSynced) - ctrl.addedPolicyObservers = true + } + + if ctrl.addedCRIOCPObservers { + listerCaches = append(listerCaches, ctrl.criocpListerSynced) } if !cache.WaitForCacheSync(stopCh, listerCaches...) { @@ -256,6 +285,9 @@ func (ctrl *Controller) Run(workers int, stopCh <-chan struct{}) { // Just need one worker for the image config go wait.Until(ctrl.imgWorker, time.Second, stopCh) + // Just need one worker for the CRIOCredentialProviderConfig + go wait.Until(ctrl.criocpWorker, time.Second, stopCh) + <-stopCh } @@ -317,6 +349,29 @@ func (ctrl *Controller) itmsConfDeleted(_ interface{}) { ctrl.imgQueue.Add("openshift-config") } +func (ctrl *Controller) addCRIOCPObservers() { + ctrl.configInformerFactory.Config().V1alpha1().CRIOCredentialProviderConfigs().Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{ + AddFunc: ctrl.criocpConfAdded, + UpdateFunc: ctrl.criocpConfUpdated, + DeleteFunc: ctrl.criocpConfDeleted, + }) + ctrl.criocpLister = ctrl.configInformerFactory.Config().V1alpha1().CRIOCredentialProviderConfigs().Lister() + ctrl.criocpListerSynced = ctrl.configInformerFactory.Config().V1alpha1().CRIOCredentialProviderConfigs().Informer().HasSynced + ctrl.addedCRIOCPObservers = true +} + +func (ctrl *Controller) criocpConfAdded(_ interface{}) { + ctrl.criocpQueue.Add("openshift-config") +} + +func (ctrl *Controller) criocpConfUpdated(_, _ interface{}) { + ctrl.criocpQueue.Add("openshift-config") +} + +func (ctrl *Controller) criocpConfDeleted(_ interface{}) { + ctrl.criocpQueue.Add("openshift-config") +} + func (ctrl *Controller) addImagePolicyObservers() { ctrl.configInformerFactory.Config().V1().ClusterImagePolicies().Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{ AddFunc: ctrl.clusterImagePolicyAdded, @@ -333,6 +388,7 @@ func (ctrl *Controller) addImagePolicyObservers() { }) ctrl.imagePolicyLister = ctrl.configInformerFactory.Config().V1().ImagePolicies().Lister() ctrl.imagePolicyListerSynced = ctrl.configInformerFactory.Config().V1().ImagePolicies().Informer().HasSynced + ctrl.addedPolicyObservers = true } func (ctrl *Controller) clusterImagePolicyAdded(_ interface{}) { @@ -363,6 +419,10 @@ func (ctrl *Controller) sigstoreAPIEnabled() bool { return ctrl.fgHandler.Enabled(features.FeatureGateSigstoreImageVerification) } +func (ctrl *Controller) criocpEnabled() bool { + return ctrl.fgHandler.Enabled(features.FeatureGateCRIOCredentialProviderConfig) +} + func (ctrl *Controller) updateContainerRuntimeConfig(oldObj, newObj interface{}) { oldCtrCfg := oldObj.(*mcfgv1.ContainerRuntimeConfig) newCtrCfg := newObj.(*mcfgv1.ContainerRuntimeConfig) @@ -442,6 +502,11 @@ func (ctrl *Controller) imgWorker() { } } +func (ctrl *Controller) criocpWorker() { + for ctrl.processNextCRIOCPWorkItem() { + } +} + func (ctrl *Controller) processNextWorkItem() bool { key, quit := ctrl.queue.Get() if quit { @@ -450,7 +515,7 @@ func (ctrl *Controller) processNextWorkItem() bool { defer ctrl.queue.Done(key) err := ctrl.syncHandler(key) - ctrl.handleErr(err, key) + ctrl.handleQueueErr(ctrl.queue, "containerruntimeconfig", err, key) return true } @@ -463,45 +528,41 @@ func (ctrl *Controller) processNextImgWorkItem() bool { defer ctrl.imgQueue.Done(key) err := ctrl.syncImgHandler(key) - ctrl.handleImgErr(err, key) + ctrl.handleQueueErr(ctrl.imgQueue, "image config", err, key) return true } -func (ctrl *Controller) handleErr(err error, key string) { - if err == nil { - ctrl.queue.Forget(key) - return +func (ctrl *Controller) processNextCRIOCPWorkItem() bool { + key, quit := ctrl.criocpQueue.Get() + if quit { + return false } + defer ctrl.criocpQueue.Done(key) - if ctrl.queue.NumRequeues(key) < maxRetries { - klog.V(2).Infof("Error syncing containerruntimeconfig %v: %v", key, err) - ctrl.queue.AddRateLimited(key) - return - } + err := ctrl.syncCRIOCPHandler(key) + ctrl.handleQueueErr(ctrl.criocpQueue, "CRIOCredentialProviderConfig", err, key) - utilruntime.HandleError(err) - klog.V(2).Infof("Dropping containerruntimeconfig %q out of the queue: %v", key, err) - ctrl.queue.Forget(key) - ctrl.queue.AddAfter(key, 1*time.Minute) + return true } -func (ctrl *Controller) handleImgErr(err error, key string) { +// handleQueueErr handles errors for a given queue and resource type. +func (ctrl *Controller) handleQueueErr(queue workqueue.TypedRateLimitingInterface[string], resourceType string, err error, key string) { if err == nil { - ctrl.imgQueue.Forget(key) + queue.Forget(key) return } - if ctrl.imgQueue.NumRequeues(key) < maxRetries { - klog.V(2).Infof("Error syncing image config %v: %v", key, err) - ctrl.imgQueue.AddRateLimited(key) + if queue.NumRequeues(key) < maxRetries { + klog.V(2).Infof("Error syncing %s %v: %v", resourceType, key, err) + queue.AddRateLimited(key) return } utilruntime.HandleError(err) - klog.V(2).Infof("Dropping image config %q out of the queue: %v", key, err) - ctrl.imgQueue.Forget(key) - ctrl.imgQueue.AddAfter(key, 1*time.Minute) + klog.V(2).Infof("Dropping %s %q out of the queue: %v", resourceType, key, err) + queue.Forget(key) + queue.AddAfter(key, 1*time.Minute) } // generateOriginalContainerRuntimeConfigs returns rendered default storage, registries and policy config files @@ -551,6 +612,53 @@ func generateOriginalContainerRuntimeConfigs(templateDir string, cc *mcfgv1.Cont return gmcStorageConfig, gmcRegistriesConfig, gmcPolicyJSON, nil } +func generateOriginalCredentialProviderConfig(templateDir string, cc *mcfgv1.ControllerConfig, role string) (*ign3types.File, string, error) { + + // Render the default templates + rc := &mtmpl.RenderConfig{ + ControllerConfigSpec: &cc.Spec, + } + generatedConfigs, err := mtmpl.GenerateMachineConfigsForRole(rc, role, templateDir) + if err != nil { + return nil, "", fmt.Errorf("generateMachineConfigsforRole failed with error %w", err) + } + + var ( + config, gmcCredProviderConfig *ign3types.File + errCredProvider error + credProviderConfigPath string + ) + + // Determine credential provider config path based on platform + // staying consistent with path used in pkg/controller/template/render.go + credProviderConfigPathFormat := filepath.FromSlash("/etc/kubernetes/credential-providers/%s-credential-provider.yaml") + switch cc.Spec.Infra.Status.PlatformStatus.Type { + case apicfgv1.AWSPlatformType: + credProviderConfigPath = fmt.Sprintf(credProviderConfigPathFormat, "ecr") + case apicfgv1.GCPPlatformType: + credProviderConfigPath = fmt.Sprintf(credProviderConfigPathFormat, "gcr") + case apicfgv1.AzurePlatformType: + credProviderConfigPath = fmt.Sprintf(credProviderConfigPathFormat, "acr") + default: + credProviderConfigPath = genericCredProviderConfigPath + return nil, credProviderConfigPath, nil + } + + // Find credential provider config + for _, gmc := range generatedConfigs { + config, errCredProvider = findCredProviderConfig(gmc, credProviderConfigPath) + if errCredProvider == nil { + gmcCredProviderConfig = config + break + } + } + if errCredProvider != nil { + return nil, "", fmt.Errorf("could not generate original credential provider configs: %w", errCredProvider) + } + + return gmcCredProviderConfig, credProviderConfigPath, nil +} + func (ctrl *Controller) syncStatusOnly(cfg *mcfgv1.ContainerRuntimeConfig, err error, args ...interface{}) error { statusUpdateErr := retry.RetryOnConflict(updateBackoff, func() error { newcfg, getErr := ctrl.mccrLister.Get(cfg.Name) @@ -1028,6 +1136,116 @@ func (ctrl *Controller) syncImageConfig(key string) error { return nil } +func (ctrl *Controller) syncCRIOCredentialProviderConfig(key string) error { + startTime := time.Now() + klog.V(4).Infof("Started syncing CRIOCredentialProvider config %q (%v)", key, startTime) + defer func() { + klog.V(4).Infof("Finished syncing CRIOCredentialProvider config %q (%v)", key, time.Since(startTime)) + }() + + if !ctrl.criocpEnabled() || !ctrl.addedCRIOCPObservers { + klog.V(2).Infof("CRIOCredentialProviderConfig observer not added, skipping sync") + return nil + } + + var ( + crioCredentialProviderConfig *apicfgv1alpha1.CRIOCredentialProviderConfig + err error + ) + + crioCredentialProviderConfig, err = ctrl.criocpLister.Get("cluster") + if errors.IsNotFound(err) { + klog.V(2).Infof("CRIOCredentialProviderConfig 'cluster' does not exist or has been deleted") + return nil + } else if err != nil { + return err + } + + // Get ControllerConfig + controllerConfig, err := ctrl.ccLister.Get(ctrlcommon.ControllerConfigName) + if err != nil { + return fmt.Errorf("could not get ControllerConfig: %w", err) + } + + sel, err := metav1.LabelSelectorAsSelector(metav1.AddLabelToSelector(&metav1.LabelSelector{}, builtInLabelKey, "")) + if err != nil { + return err + } + // Find all the MCO built in MachineConfigPools + mcpPools, err := ctrl.mcpLister.List(sel) + if err != nil { + return err + } + + for _, pool := range mcpPools { + applied := true + + managedKeyCredentialProvider, err := getManagedKeyCRIOCredentialProvider(pool) + if err != nil { + ctrl.syncCRIOCredentialProviderConfigStatusOnly(err, apicfgv1alpha1.ConditionTypeMachineConfigRendered, apicfgv1alpha1.ReasonMachineConfigRenderingFailed, "could not get CRIOCredentialProviderConfig managed key: %v", err) + return err + } + + if err := retry.RetryOnConflict(updateBackoff, func() error { + + credentialProviderConfigIgn, overlappedEntries, err := crioCredentialProviderConfigIgnition(ctrl.templatesDir, controllerConfig, pool.Name, crioCredentialProviderConfig) + if err != nil { + ctrl.syncCRIOCredentialProviderConfigStatusOnly(err, apicfgv1alpha1.ConditionTypeMachineConfigRendered, apicfgv1alpha1.ReasonMachineConfigRenderingFailed, "could not generate CRIOCredentialProvider Ignition config: %v", err) + return err + } + if len(overlappedEntries) > 0 { + ctrl.syncCRIOCredentialProviderConfigStatusOnly(nil, apicfgv1alpha1.ConditionTypeValidated, apicfgv1alpha1.ReasonConfigurationPartiallyApplied, "CRIOCredentialProviderConfig has one or multiple entries that overlap with the original credential provider config. Skip rendering entries: %v.", overlappedEntries) + } else { + ctrl.syncCRIOCredentialProviderConfigStatusOnly(nil, apicfgv1alpha1.ConditionTypeValidated, "") + } + + applied, err = ctrl.syncIgnitionConfig(managedKeyCredentialProvider, credentialProviderConfigIgn, pool, ownerReferenceCredentialProviderConfig(crioCredentialProviderConfig)) + if err != nil { + ctrl.syncCRIOCredentialProviderConfigStatusOnly(err, apicfgv1alpha1.ConditionTypeMachineConfigRendered, apicfgv1alpha1.ReasonMachineConfigRenderingFailed, "could not sync CRIOCredentialProvider Ignition config: %v", err) + return err + } + + return nil + }); err != nil { + ctrl.syncCRIOCredentialProviderConfigStatusOnly(err, apicfgv1alpha1.ConditionTypeMachineConfigRendered, apicfgv1alpha1.ReasonMachineConfigRenderingFailed, "could not Create/Update MachineConfig: %v", err) + return err + } + + if applied { + klog.Infof("Applied CRIOCredentialProviderConfig cluster on MachineConfigPool %v", pool.Name) + ctrlcommon.UpdateStateMetric(ctrlcommon.MCCSubControllerState, "machine-config-controller-container-runtime-config", "Sync CRIO Credential Provider Config", pool.Name) + } + ctrl.syncCRIOCredentialProviderConfigStatusOnly(nil, apicfgv1alpha1.ConditionTypeMachineConfigRendered, apicfgv1alpha1.ReasonMachineConfigRenderingSucceeded) + } + + return nil +} + +func (ctrl *Controller) syncCRIOCredentialProviderConfigStatusOnly(err error, conditionType, reason string, args ...interface{}) { + statusUpdateErr := retry.RetryOnConflict(updateBackoff, func() error { + newCfg, getErr := ctrl.criocpLister.Get("cluster") + if getErr != nil { + return getErr + } + newCfg = newCfg.DeepCopy() + + newStatusCondition := wrapErrorWithCRIOCredentialProviderConfigCondition(err, conditionType, reason, args...) + + // Update the observedGeneration + if newCfg.GetGeneration() != newStatusCondition.ObservedGeneration { + newStatusCondition.ObservedGeneration = newCfg.GetGeneration() + } + + meta.SetStatusCondition(&newCfg.Status.Conditions, newStatusCondition) + _, updateErr := ctrl.configClient.ConfigV1alpha1().CRIOCredentialProviderConfigs().UpdateStatus(context.TODO(), newCfg, metav1.UpdateOptions{}) + return updateErr + }) + // If an error occurred in updating the status just log it + if statusUpdateErr != nil { + klog.Warningf("error updating CRIOCredentialProviderConfig status: %v", statusUpdateErr) + } +} + func (ctrl *Controller) syncIgnitionConfig(managedKey string, ignFile *ign3types.Config, pool *mcfgv1.MachineConfigPool, ownerRef metav1.OwnerReference) (bool, error) { rawIgn, err := json.Marshal(ignFile) if err != nil { @@ -1372,3 +1590,45 @@ func (ctrl *Controller) getPoolsForContainerRuntimeConfig(config *mcfgv1.Contain return pools, nil } + +func crioCredentialProviderConfigIgnition(templateDir string, controllerConfig *mcfgv1.ControllerConfig, role string, crioCredentialProviderConfig *apicfgv1alpha1.CRIOCredentialProviderConfig) (*ign3types.Config, []string, error) { + + var ( + credProviderConfigYaml []byte + overlappedEntries []string + CredProviderConfigDropInUnitToml []byte + contents []byte + ) + + originalCredProviderConfigIgn, credProviderConfigPath, err := generateOriginalCredentialProviderConfig(templateDir, controllerConfig, role) + if err != nil { + return nil, nil, fmt.Errorf("could not generate original CRIOCredentialProvider config for role %s: %w", role, err) + } + if originalCredProviderConfigIgn != nil { + contents, err = ctrlcommon.DecodeIgnitionFileContents(originalCredProviderConfigIgn.Contents.Source, originalCredProviderConfigIgn.Contents.Compression) + if err != nil { + return nil, nil, fmt.Errorf("could not decode CRIOCredentialProvider config for role %s: %w", role, err) + } + } + + credProviderConfigObject, err := credProviderConfigObject(contents) + if err != nil { + return nil, nil, err + } + + if len(crioCredentialProviderConfig.Spec.MatchImages) != 0 { + credProviderConfigYaml, overlappedEntries, err = updateCredentialProviderConfig(credProviderConfigObject, crioCredentialProviderConfig.Spec.MatchImages) + if err != nil { + return nil, nil, err + } + + if credProviderConfigYaml != nil && credProviderConfigPath == genericCredProviderConfigPath { + CredProviderConfigDropInUnitToml = generateDropinUnitCredProviderConfig(genericCredProviderConfigPath) + } + } + credProviderConfigIgn := createNewIgnition([]generatedConfigFile{ + {filePath: credProviderConfigPath, data: credProviderConfigYaml}, + {filePath: constants.KubeletCrioImageCredProviderConfPath, data: CredProviderConfigDropInUnitToml}, + }) + return &credProviderConfigIgn, overlappedEntries, nil +} diff --git a/pkg/controller/container-runtime-config/container_runtime_config_controller_test.go b/pkg/controller/container-runtime-config/container_runtime_config_controller_test.go index 0b25112b01..d83976d464 100644 --- a/pkg/controller/container-runtime-config/container_runtime_config_controller_test.go +++ b/pkg/controller/container-runtime-config/container_runtime_config_controller_test.go @@ -30,6 +30,7 @@ import ( ign3types "github.com/coreos/ignition/v2/config/v3_5/types" apicfgv1 "github.com/openshift/api/config/v1" + apicfgv1alpha1 "github.com/openshift/api/config/v1alpha1" features "github.com/openshift/api/features" mcfgv1 "github.com/openshift/api/machineconfiguration/v1" apioperatorsv1alpha1 "github.com/openshift/api/operator/v1alpha1" @@ -75,6 +76,7 @@ type fixture struct { itmsLister []*apicfgv1.ImageTagMirrorSet clusterImagePolicyLister []*apicfgv1.ClusterImagePolicy imagePolicyLister []*apicfgv1.ImagePolicy + criocpLister []*apicfgv1alpha1.CRIOCredentialProviderConfig actions []core.Action skipActionsValidation bool @@ -91,7 +93,10 @@ func newFixture(t *testing.T) *fixture { f.t = t f.objects = []runtime.Object{} f.fgHandler = ctrlcommon.NewFeatureGatesHardcodedHandler( - []apicfgv1.FeatureGateName{features.FeatureGateSigstoreImageVerification}, + []apicfgv1.FeatureGateName{ + features.FeatureGateSigstoreImageVerification, + features.FeatureGateCRIOCredentialProviderConfig, + }, []apicfgv1.FeatureGateName{}, ) return f @@ -286,6 +291,7 @@ func (f *fixture) newController() *Controller { c.clusterImagePolicyListerSynced = alwaysReady c.imagePolicyListerSynced = alwaysReady c.clusterVersionListerSynced = alwaysReady + c.criocpListerSynced = alwaysReady c.eventRecorder = &record.FakeRecorder{} stopCh := make(chan struct{}) @@ -327,6 +333,9 @@ func (f *fixture) newController() *Controller { for _, c := range f.imagePolicyLister { ci.Config().V1().ImagePolicies().Informer().GetIndexer().Add(c) } + for _, c := range f.criocpLister { + ci.Config().V1alpha1().CRIOCredentialProviderConfigs().Informer().GetIndexer().Add(c) + } return c } @@ -345,6 +354,10 @@ func (f *fixture) runController(mcpname string, expectError bool) { c.addImagePolicyObservers() c.addedPolicyObservers = true } + if !c.addedCRIOCPObservers { + c.addCRIOCPObservers() + c.addedCRIOCPObservers = true + } err := c.syncImgHandler(mcpname) if !expectError && err != nil { f.t.Errorf("error syncing image config: %v", err) @@ -359,6 +372,13 @@ func (f *fixture) runController(mcpname string, expectError bool) { f.t.Error("expected error syncing containerruntimeconfigs, got nil") } + err = c.syncCRIOCPHandler(mcpname) + if !expectError && err != nil { + f.t.Errorf("error syncing CRIOCredentialProviderConfigs: %v", err) + } else if expectError && err == nil { + f.t.Error("expected error syncing CRIOCredentialProviderConfigs, got nil") + } + f.validateActions() } @@ -622,6 +642,85 @@ func verifyRegistriesConfigAndPolicyJSONContents(t *testing.T, mc *mcfgv1.Machin } } +type criocpConfigVerifyOptions struct { + expectDropin bool + expectDropinPath string + expectMCNilContent bool +} + +func (f *fixture) verifyCRIOCredentialProviderConfigContents(t *testing.T, mcName string, criocp *apicfgv1alpha1.CRIOCredentialProviderConfig, verifyOpts criocpConfigVerifyOptions) { + updatedMC, err := f.client.MachineconfigurationV1().MachineConfigs().Get(context.TODO(), mcName, metav1.GetOptions{}) + require.NoError(t, err) + + ignCfg, err := ctrlcommon.ParseAndConvertConfig(updatedMC.Spec.Config.Raw) + require.NoError(t, err) + + if verifyOpts.expectMCNilContent { + if len(ignCfg.Storage.Files) != 0 { + t.Fatalf("expected no file in the CRIO credential provider config machineconfig") + } + return + } + + // find any file under /etc/kubernetes/credential-providers and validate it contains the crio provider entry + // under the precondition that the criocp.Spec.MatchImages should have at least one entry + found := false + for _, ffile := range ignCfg.Storage.Files { + if !strings.HasPrefix(ffile.Path, constants.KubernetesCredentialProvidersDir) { + continue + } + + contents, err := ctrlcommon.DecodeIgnitionFileContents(ffile.Contents.Source, ffile.Contents.Compression) + require.NoError(t, err) + + credObj, err := credProviderConfigObject(contents) + require.NoError(t, err) + + for _, provider := range credObj.Providers { + if provider.Name != crioCredentialProviderName { + continue + } + + allMatch := true + for _, mi := range criocp.Spec.MatchImages { + if !assert.Contains(t, provider.MatchImages, string(mi)) { + allMatch = false + break + } + } + + found = allMatch + break + } + + if found { + break + } + } + if !found { + t.Fatalf("did not find %s provider with expected matchImages in MachineConfig %s", crioCredentialProviderName, mcName) + } + + if verifyOpts.expectDropin { + // If the CRIOCredentialProviderConfig has MatchImages then we expect a drop-in file to be created in the MC for it. Validate that it exists and contains the expected content. + expectedDropinConfig := generateDropinUnitCredProviderConfig(genericCredProviderConfigPath) + foundDropin := false + for _, ffile := range ignCfg.Storage.Files { + if ffile.Node.Path != verifyOpts.expectDropinPath { + continue + } + + foundDropin = true + dropinContents, err := ctrlcommon.DecodeIgnitionFileContents(ffile.Contents.Source, ffile.Contents.Compression) + require.NoError(t, err) + assert.Equal(t, string(expectedDropinConfig), string(dropinContents)) + } + if !foundDropin { + t.Errorf("Expected to find credential provider drop-in file in machineconfig %s", mcName) + } + } +} + // The patch bytes to expect when creating/updating a containerruntimeconfig var ctrcfgPatchBytes = []byte("{\"metadata\":{\"finalizers\":[\"99-master-generated-containerruntime\"]}}") @@ -2026,3 +2125,199 @@ func TestImagePolicyCreate(t *testing.T) { }) } } + +func TestCrioCredentialProviderConfigCreate(t *testing.T) { + for _, platform := range []apicfgv1.PlatformType{apicfgv1.AWSPlatformType, + apicfgv1.GCPPlatformType, + apicfgv1.AzurePlatformType, + apicfgv1.NonePlatformType, + } { + t.Run(string(platform), func(t *testing.T) { + f := newFixture(t) + f.newController() + + cc := newControllerConfig(ctrlcommon.ControllerConfigName, platform) + mcp := helpers.NewMachineConfigPool("master", nil, helpers.MasterSelector, "v0") + mcp1 := helpers.NewMachineConfigPool("worker", nil, helpers.WorkerSelector, "v0") + criocp := newCrioCredentialProviderConfig("cluster", []string{"example.com"}) + + cvcfg1 := newClusterVersionConfig("version", "test.io/myuser/myimage:test") + keyCRIOCP, _ := getManagedKeyCRIOCredentialProvider(mcp) + keyCRIOCP1, _ := getManagedKeyCRIOCredentialProvider(mcp1) + + mcs := helpers.NewMachineConfig(keyCRIOCP, map[string]string{"node-role": "master"}, "dummy://", []ign3types.File{{}}) + mcs1 := helpers.NewMachineConfig(keyCRIOCP1, map[string]string{"node-role": "worker"}, "dummy://", []ign3types.File{{}}) + f.ccLister = append(f.ccLister, cc) + f.mcpLister = append(f.mcpLister, mcp) + f.mcpLister = append(f.mcpLister, mcp1) + f.criocpLister = append(f.criocpLister, criocp) + f.cvLister = append(f.cvLister, cvcfg1) + f.imgObjects = append(f.imgObjects, criocp) + + f.expectGetMachineConfigAction(mcs) + f.expectCreateMachineConfigAction(mcs) + + f.expectGetMachineConfigAction(mcs1) + f.expectCreateMachineConfigAction(mcs1) + + f.run("") + + verifyOpts := criocpConfigVerifyOptions{} + + if platform != apicfgv1.AWSPlatformType && platform != apicfgv1.GCPPlatformType && platform != apicfgv1.AzurePlatformType { + verifyOpts.expectDropin = true + verifyOpts.expectDropinPath = constants.KubeletCrioImageCredProviderConfPath + } + + f.verifyCRIOCredentialProviderConfigContents(t, mcs.Name, criocp, verifyOpts) + }) + } +} + +func TestCrioCredentialProviderConfigUpdate(t *testing.T) { + for _, platform := range []apicfgv1.PlatformType{apicfgv1.AWSPlatformType, + apicfgv1.GCPPlatformType, + apicfgv1.AzurePlatformType, + apicfgv1.NonePlatformType, + } { + t.Run(string(platform), func(t *testing.T) { + f := newFixture(t) + f.newController() + + cc := newControllerConfig(ctrlcommon.ControllerConfigName, platform) + mcp := helpers.NewMachineConfigPool("master", nil, helpers.MasterSelector, "v0") + mcp1 := helpers.NewMachineConfigPool("worker", nil, helpers.WorkerSelector, "v0") + criocp := newCrioCredentialProviderConfig("cluster", []string{"example.com"}) + + cvcfg1 := newClusterVersionConfig("version", "test.io/myuser/myimage:test") + keyCRIOCP, _ := getManagedKeyCRIOCredentialProvider(mcp) + keyCRIOCP1, _ := getManagedKeyCRIOCredentialProvider(mcp1) + + mcs := helpers.NewMachineConfig(keyCRIOCP, map[string]string{"node-role": "master"}, "dummy://", []ign3types.File{{}}) + mcs1 := helpers.NewMachineConfig(keyCRIOCP1, map[string]string{"node-role": "worker"}, "dummy://", []ign3types.File{{}}) + f.ccLister = append(f.ccLister, cc) + f.mcpLister = append(f.mcpLister, mcp) + f.mcpLister = append(f.mcpLister, mcp1) + f.criocpLister = append(f.criocpLister, criocp) + f.cvLister = append(f.cvLister, cvcfg1) + f.imgObjects = append(f.imgObjects, criocp) // criocp is added to config objects to make sure the informer can get it + + mcsUpdate := mcs.DeepCopy() + mcs1Update := mcs1.DeepCopy() + + f.expectGetMachineConfigAction(mcs) + f.expectCreateMachineConfigAction(mcs) + + f.expectGetMachineConfigAction(mcs1) + f.expectCreateMachineConfigAction(mcs1) + + stopCh := make(chan struct{}) + + f.run("") + + f.validateActions() + close(stopCh) + + verifyOpts := criocpConfigVerifyOptions{} + if platform != apicfgv1.AWSPlatformType && platform != apicfgv1.GCPPlatformType && platform != apicfgv1.AzurePlatformType { + verifyOpts.expectDropin = true + verifyOpts.expectDropinPath = constants.KubeletCrioImageCredProviderConfPath + } + + f.verifyCRIOCredentialProviderConfigContents(t, mcs.Name, criocp, verifyOpts) + + f = newFixture(t) + criocpUpdate := criocp.DeepCopy() + criocpUpdate.Spec.MatchImages = []apicfgv1alpha1.MatchImage{"example.com", "example1.com"} + + f.criocpLister = append(f.criocpLister, criocpUpdate) + f.ccLister = append(f.ccLister, cc) + f.mcpLister = append(f.mcpLister, mcp) + f.mcpLister = append(f.mcpLister, mcp1) + f.cvLister = append(f.cvLister, cvcfg1) + f.objects = append(f.objects, mcsUpdate, mcs1Update) + f.imgObjects = append(f.imgObjects, criocpUpdate) + + f.expectGetMachineConfigAction(mcsUpdate) + f.expectUpdateMachineConfigAction(mcsUpdate) + + f.expectGetMachineConfigAction(mcs1Update) + f.expectUpdateMachineConfigAction(mcs1Update) + + stopCh = make(chan struct{}) + + f.run("") + + f.validateActions() + close(stopCh) + + f.verifyCRIOCredentialProviderConfigContents(t, mcs.Name, criocpUpdate, verifyOpts) + + }) + } +} + +func newCrioCredentialProviderConfig(name string, matchImages []string) *apicfgv1alpha1.CRIOCredentialProviderConfig { + var images []apicfgv1alpha1.MatchImage + for _, img := range matchImages { + images = append(images, apicfgv1alpha1.MatchImage(img)) + } + + return &apicfgv1alpha1.CRIOCredentialProviderConfig{ + TypeMeta: metav1.TypeMeta{ + APIVersion: apicfgv1alpha1.SchemeGroupVersion.String(), + Kind: "CrioCredentialProviderConfig", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: name, + }, + Spec: &apicfgv1alpha1.CRIOCredentialProviderConfigSpec{ + MatchImages: images, + }, + } +} + +func TestCrioCredentialProviderConfigCreateEmpty(t *testing.T) { + for _, platform := range []apicfgv1.PlatformType{apicfgv1.AWSPlatformType, + apicfgv1.GCPPlatformType, + apicfgv1.AzurePlatformType, + apicfgv1.NonePlatformType, + } { + t.Run(string(platform), func(t *testing.T) { + f := newFixture(t) + f.newController() + + cc := newControllerConfig(ctrlcommon.ControllerConfigName, platform) + mcp := helpers.NewMachineConfigPool("master", nil, helpers.MasterSelector, "v0") + mcp1 := helpers.NewMachineConfigPool("worker", nil, helpers.WorkerSelector, "v0") + criocp := newCrioCredentialProviderConfig("cluster", nil) + + cvcfg1 := newClusterVersionConfig("version", "test.io/myuser/myimage:test") + keyCRIOCP, _ := getManagedKeyCRIOCredentialProvider(mcp) + keyCRIOCP1, _ := getManagedKeyCRIOCredentialProvider(mcp1) + + mcs := helpers.NewMachineConfig(keyCRIOCP, map[string]string{"node-role": "master"}, "dummy://", []ign3types.File{{}}) + mcs1 := helpers.NewMachineConfig(keyCRIOCP1, map[string]string{"node-role": "worker"}, "dummy://", []ign3types.File{{}}) + f.ccLister = append(f.ccLister, cc) + f.mcpLister = append(f.mcpLister, mcp) + f.mcpLister = append(f.mcpLister, mcp1) + f.criocpLister = append(f.criocpLister, criocp) + f.cvLister = append(f.cvLister, cvcfg1) + f.imgObjects = append(f.imgObjects, criocp) + + f.expectGetMachineConfigAction(mcs) + f.expectCreateMachineConfigAction(mcs) + + f.expectGetMachineConfigAction(mcs1) + f.expectCreateMachineConfigAction(mcs1) + + f.run("") + + verifyOpts := criocpConfigVerifyOptions{ + expectMCNilContent: true, + } + + f.verifyCRIOCredentialProviderConfigContents(t, mcs.Name, criocp, verifyOpts) + }) + } +} diff --git a/pkg/controller/container-runtime-config/helpers.go b/pkg/controller/container-runtime-config/helpers.go index 85245534ca..4ff7784c14 100644 --- a/pkg/controller/container-runtime-config/helpers.go +++ b/pkg/controller/container-runtime-config/helpers.go @@ -13,6 +13,7 @@ import ( "sort" "strconv" "strings" + "time" "github.com/BurntSushi/toml" "github.com/containers/image/v5/docker/reference" @@ -24,6 +25,7 @@ import ( "github.com/ghodss/yaml" "github.com/opencontainers/go-digest" apicfgv1 "github.com/openshift/api/config/v1" + apicfgv1alpha1 "github.com/openshift/api/config/v1alpha1" apioperatorsv1alpha1 "github.com/openshift/api/operator/v1alpha1" "github.com/openshift/runtime-utils/pkg/registries" runtimeutils "github.com/openshift/runtime-utils/pkg/registries" @@ -37,6 +39,8 @@ import ( "github.com/openshift/machine-config-operator/pkg/apihelpers" ctrlcommon "github.com/openshift/machine-config-operator/pkg/controller/common" "github.com/openshift/machine-config-operator/pkg/daemon/constants" + kubeletconfig "k8s.io/kubernetes/pkg/kubelet/apis/config" + "k8s.io/utils/ptr" ) const ( @@ -53,8 +57,9 @@ const ( crioDropInFilePathPidsLimit = "/etc/crio/crio.conf.d/01-ctrcfg-pidsLimit" crioDropInFilePathLogSizeMax = "/etc/crio/crio.conf.d/01-ctrcfg-logSizeMax" CRIODropInFilePathDefaultRuntime = "/etc/crio/crio.conf.d/01-ctrcfg-defaultRuntime" - imagepolicyType = "sigstoreSigned" sigstoreRegistriesConfigFilePath = "/etc/containers/registries.d/sigstore-registries.yaml" + crioCredentialProviderName = "crio-credential-provider" + credentialProviderAPIVersion = "credentialprovider.kubelet.k8s.io/v1" ) var ( @@ -200,6 +205,19 @@ func findPolicyJSON(mc *mcfgv1.MachineConfig) (*ign3types.File, error) { return nil, fmt.Errorf("could not find Policy JSON") } +func findCredProviderConfig(mc *mcfgv1.MachineConfig, credProviderConfigPath string) (*ign3types.File, error) { + ignCfg, err := ctrlcommon.ParseAndConvertConfig(mc.Spec.Config.Raw) + if err != nil { + return nil, fmt.Errorf("parsing CRIOCredentialProvider Ignition config failed with error: %w", err) + } + for _, c := range ignCfg.Storage.Files { + if c.Path == credProviderConfigPath { + return &c, nil + } + } + return nil, fmt.Errorf("could not find CRIOCredentialProvider Config") +} + // Deprecated: use getManagedKeyCtrCfg func getManagedKeyCtrCfgDeprecated(pool *mcfgv1.MachineConfigPool) string { return fmt.Sprintf("99-%s-%s-containerruntime", pool.Name, pool.ObjectMeta.UID) @@ -336,6 +354,10 @@ func notLatestContainerRuntimeConfigInPool(ctrcfgList []mcfgv1.ContainerRuntimeC return false } +func getManagedKeyCRIOCredentialProvider(pool *mcfgv1.MachineConfigPool) (string, error) { + return ctrlcommon.GetManagedKey(pool, nil, "97", "credentialproviderconfig", "") +} + // Deprecated: use getManagedKeyReg func getManagedKeyRegDeprecated(pool *mcfgv1.MachineConfigPool) string { return fmt.Sprintf("99-%s-%s-registries", pool.Name, pool.ObjectMeta.UID) @@ -852,6 +874,15 @@ func ownerReferenceImageConfig(imageConfig *apicfgv1.Image) metav1.OwnerReferenc } } +func ownerReferenceCredentialProviderConfig(credentialProviderConfig *apicfgv1alpha1.CRIOCredentialProviderConfig) metav1.OwnerReference { + return metav1.OwnerReference{ + APIVersion: apicfgv1alpha1.SchemeGroupVersion.String(), + Kind: "CRIOCredentialProviderConfig", + Name: credentialProviderConfig.Name, + UID: credentialProviderConfig.UID, + } +} + func policyItemFromSpec(policy apicfgv1.ImageSigstoreVerificationPolicy) (signature.PolicyRequirement, error) { var ( sigstorePolicyRequirement signature.PolicyRequirement @@ -1211,3 +1242,205 @@ func imagePolicyConfigFileList(namespaceJSONs map[string][]byte) []generatedConf } return namespacedPolicyConfigFileList } + +func credProviderConfigObject(contents []byte) (*credentialProviderConfigWithVersion, error) { + // Unmarshal into custom struct first to handle YAML with omitempty fields + credProviderConfigObject := &credentialProviderConfigWithVersion{ + APIVersion: "kubelet.config.k8s.io/v1", + Kind: "CredentialProviderConfig", + } + err := yaml.Unmarshal(contents, credProviderConfigObject) + if err != nil { + return nil, fmt.Errorf("error unmarshalling credential provider config: %w", err) + } + + return credProviderConfigObject, nil +} + +// credentialProviderWithTag is a custom struct with omitempty tags to avoid null values in YAML +type credentialProviderWithTag struct { + Name string `json:"name"` + MatchImages []string `json:"matchImages"` + DefaultCacheDuration *metav1.Duration `json:"defaultCacheDuration,omitempty"` + APIVersion string `json:"apiVersion"` + Args []string `json:"args,omitempty"` + Env []kubeletconfig.ExecEnvVar `json:"env,omitempty"` + TokenAttributes *serviceAccountTokenAttributesVersioned `json:"tokenAttributes,omitempty"` +} + +// serviceAccountTokenAttributesVersioned is a custom struct with omitempty tags to avoid null values in YAML +type serviceAccountTokenAttributesVersioned struct { + ServiceAccountTokenAudience string `json:"serviceAccountTokenAudience"` + CacheType kubeletconfig.ServiceAccountTokenCacheType `json:"cacheType"` + RequireServiceAccount *bool `json:"requireServiceAccount"` + RequiredServiceAccountAnnotationKeys []string `json:"requiredServiceAccountAnnotationKeys,omitempty"` + OptionalServiceAccountAnnotationKeys []string `json:"optionalServiceAccountAnnotationKeys,omitempty"` +} + +type credentialProviderConfigWithVersion struct { + APIVersion string `json:"apiVersion"` + Kind string `json:"kind"` + Providers []*credentialProviderWithTag `json:"providers"` +} + +func updateCredentialProviderConfig(credProviderConfigObject *credentialProviderConfigWithVersion, newImages []apicfgv1alpha1.MatchImage) ([]byte, []string, error) { + + // newImages is not expected to be empty here as the caller should skip calling this function if there are no images + var ( + overlappedEntries = make(map[string]bool) + conflictList []string + images []string + matchImages = make(map[string]bool) + ) + + crioCredProviderExist := false + crioCredProviderIdx := -1 + + for i, provider := range credProviderConfigObject.Providers { + + if provider.Name != crioCredentialProviderName { + continue + } + + crioCredProviderExist = true + crioCredProviderIdx = i + break + } + + for _, newImg := range newImages { + img := string(newImg) + conflicted := false + + for _, provider := range credProviderConfigObject.Providers { + if provider.Name == crioCredentialProviderName { + continue + } + for _, existingImg := range provider.MatchImages { + if runtimeutils.ScopeIsNestedInsideScope(img, existingImg) || runtimeutils.ScopeIsNestedInsideScope(existingImg, img) { + overlappedEntries[img] = true + conflicted = true + break + } + } + if conflicted { + break + } + } + + if !conflicted { + matchImages[img] = true + } + } + + for img := range overlappedEntries { + conflictList = append(conflictList, img) + } + sort.Strings(conflictList) + + if len(matchImages) == 0 && len(overlappedEntries) > 0 { + return nil, conflictList, nil + } + + if len(matchImages) > 0 { + for img := range matchImages { + images = append(images, img) + } + } else if len(overlappedEntries) == 0 { + // No existing providers to conflict with (e.g. non-cloud platform) + for _, img := range newImages { + images = append(images, string(img)) + } + } + sort.Strings(images) + + if crioCredProviderExist && crioCredProviderIdx != -1 { + credProviderConfigObject.Providers[crioCredProviderIdx].MatchImages = images + } else { + newProvider := &credentialProviderWithTag{ + Name: crioCredentialProviderName, + MatchImages: images, + DefaultCacheDuration: &metav1.Duration{Duration: time.Second}, + APIVersion: credentialProviderAPIVersion, + TokenAttributes: &serviceAccountTokenAttributesVersioned{ + ServiceAccountTokenAudience: "https://kubernetes.default.svc", + RequireServiceAccount: ptr.To(false), + CacheType: kubeletconfig.TokenServiceAccountTokenCacheType, + }, + } + credProviderConfigObject.Providers = append(credProviderConfigObject.Providers, newProvider) + } + + credProviderConfigsYaml, err := yaml.Marshal(credProviderConfigObject) + if err != nil { + return nil, nil, fmt.Errorf("error marshalling credential provider config: %v", err) + } + + return credProviderConfigsYaml, conflictList, nil +} + +// generateDropinUnitCredProviderConfig generates the systemd drop-in unit file content +// for kubelet credential provider configuration. This configures the kubelet with the +// image credential provider config path via environment variables. +func generateDropinUnitCredProviderConfig(genericCredProviderConfigPath string) []byte { + credentialProviderBinDirFlag := "--image-credential-provider-bin-dir=/usr/libexec/kubelet-image-credential-provider-plugins" + dropInContent := fmt.Sprintf(`[Service] +# Prepends the credential provider flags to the existing Kubelet arguments +Environment="KUBELET_CRIO_IMAGE_CREDENTIAL_PROVIDER_FLAGS=%s --image-credential-provider-config=%s" +`, credentialProviderBinDirFlag, genericCredProviderConfigPath) + return []byte(dropInContent) +} + +func wrapErrorWithCRIOCredentialProviderConfigCondition(err error, conditionType, reason string, args ...interface{}) metav1.Condition { + message := "" + if len(args) >= 1 { + if format, ok := args[0].(string); ok { + message = format + if len(args) > 1 { + message = fmt.Sprintf(format, args[1:]...) + } + } + } + + if conditionType == "" { + conditionType = apicfgv1alpha1.ConditionTypeMachineConfigRendered + } + + if err == nil { + if reason == "" { + if conditionType == apicfgv1alpha1.ConditionTypeMachineConfigRendered { + reason = apicfgv1alpha1.ReasonMachineConfigRenderingSucceeded + } else { + reason = "Succeeded" + } + } + if message == "" { + message = "Success" + } + + status := metav1.ConditionTrue + if conditionType == apicfgv1alpha1.ConditionTypeValidated && reason == apicfgv1alpha1.ReasonConfigurationPartiallyApplied { + status = metav1.ConditionFalse + } + + return *apihelpers.NewCondition( + conditionType, + status, + reason, + message, + ) + } + + if reason == "" { + reason = apicfgv1alpha1.ReasonMachineConfigRenderingFailed + } + if message == "" { + message = fmt.Sprintf("Error: %v", err) + } + + return *apihelpers.NewCondition( + conditionType, + metav1.ConditionFalse, + reason, + message, + ) +} diff --git a/pkg/controller/container-runtime-config/helpers_test.go b/pkg/controller/container-runtime-config/helpers_test.go index 9d6c7b38e8..dc8a2a6856 100644 --- a/pkg/controller/container-runtime-config/helpers_test.go +++ b/pkg/controller/container-runtime-config/helpers_test.go @@ -9,6 +9,7 @@ import ( "os" "reflect" "testing" + "time" "github.com/BurntSushi/toml" "github.com/containers/image/v5/pkg/sysregistriesv2" @@ -16,6 +17,7 @@ import ( "github.com/containers/image/v5/types" storageconfig "github.com/containers/storage/pkg/config" apicfgv1 "github.com/openshift/api/config/v1" + apicfgv1alpha1 "github.com/openshift/api/config/v1alpha1" mcfgv1 "github.com/openshift/api/machineconfiguration/v1" apioperatorsv1alpha1 "github.com/openshift/api/operator/v1alpha1" "github.com/stretchr/testify/assert" @@ -26,6 +28,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/diff" "k8s.io/apimachinery/pkg/util/yaml" + "k8s.io/utils/ptr" ) func TestUpdateRegistriesConfig(t *testing.T) { @@ -2299,3 +2302,303 @@ func TestWrapErrorWithCondition(t *testing.T) { }) } } + +func TestWrapErrorWithCRIOCredentialProviderConfigCondition(t *testing.T) { + tests := []struct { + name string + err error + conditionType string + reason string + args []interface{} + expectedType string + expectedStatus metav1.ConditionStatus + expectedReason string + expectedMsg string + }{ + { + name: "nil error defaults to rendered success", + err: nil, + conditionType: "", + reason: "", + args: nil, + expectedType: apicfgv1alpha1.ConditionTypeMachineConfigRendered, + expectedStatus: metav1.ConditionTrue, + expectedReason: apicfgv1alpha1.ReasonMachineConfigRenderingSucceeded, + expectedMsg: "Success", + }, + { + name: "validated partial applied returns false status", + err: nil, + conditionType: apicfgv1alpha1.ConditionTypeValidated, + reason: apicfgv1alpha1.ReasonConfigurationPartiallyApplied, + args: []interface{}{"partially applied"}, + expectedType: apicfgv1alpha1.ConditionTypeValidated, + expectedStatus: metav1.ConditionFalse, + expectedReason: apicfgv1alpha1.ReasonConfigurationPartiallyApplied, + expectedMsg: "partially applied", + }, + { + name: "validated success returns true status", + err: nil, + conditionType: apicfgv1alpha1.ConditionTypeValidated, + reason: "", + expectedType: apicfgv1alpha1.ConditionTypeValidated, + expectedStatus: metav1.ConditionTrue, + expectedReason: "Succeeded", + expectedMsg: "Success", + }, + { + name: "error defaults reason to machine config rendering failed", + err: errors.New("boom"), + conditionType: apicfgv1alpha1.ConditionTypeValidated, + reason: "", + args: nil, + expectedType: apicfgv1alpha1.ConditionTypeValidated, + expectedStatus: metav1.ConditionFalse, + expectedReason: apicfgv1alpha1.ReasonMachineConfigRenderingFailed, + expectedMsg: "Error: boom", + }, + { + name: "error with explicit reason and message format", + err: errors.New("bad input"), + conditionType: apicfgv1alpha1.ConditionTypeMachineConfigRendered, + reason: apicfgv1alpha1.ReasonValidationFailed, + args: []interface{}{"custom %s", "message"}, + expectedType: apicfgv1alpha1.ConditionTypeMachineConfigRendered, + expectedStatus: metav1.ConditionFalse, + expectedReason: apicfgv1alpha1.ReasonValidationFailed, + expectedMsg: "custom message", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + condition := wrapErrorWithCRIOCredentialProviderConfigCondition(tt.err, tt.conditionType, tt.reason, tt.args...) + + assert.Equal(t, tt.expectedType, condition.Type) + assert.Equal(t, tt.expectedStatus, condition.Status) + assert.Equal(t, tt.expectedReason, condition.Reason) + assert.Equal(t, tt.expectedMsg, condition.Message) + }) + } +} + +func TestUpdateCredentialProviderConfig(t *testing.T) { + templateCredProviderConfig := []byte(`apiVersion: kubelet.config.k8s.io/v1 +kind: CredentialProviderConfig +providers: + - name: gcr-credential-provider + apiVersion: credentialprovider.kubelet.k8s.io/v1 + matchImages: + - "gcr.io" + - "*.gcr.io" + - "*.pkg.dev" + - "container.cloud.google.com" + defaultCacheDuration: "1m" + args: + - get-credentials + - --v=3 +`) + + templateCredProviderConfigWithCRIOProvider := []byte(`apiVersion: kubelet.config.k8s.io/v1 +kind: CredentialProviderConfig +providers: + - name: gcr-credential-provider + apiVersion: credentialprovider.kubelet.k8s.io/v1 + matchImages: + - "gcr.io" + - "*.gcr.io" + - "*.pkg.dev" + - "container.cloud.google.com" + defaultCacheDuration: "1m" + args: + - get-credentials + - --v=3 + - name: crio-credential-provider + matchImages: + - "docker.io" + - "*.example.io" + - "quay.io" + - "registry.example.com:5000" + defaultCacheDuration: "1s" + apiVersion: credentialprovider.kubelet.k8s.io/v1 + tokenAttributes: + serviceAccountTokenAudience: https://kubernetes.default.svc + cacheType: "Token" + requireServiceAccount: false +`) + + tests := []struct { + name string + matchImages []string + templateConfig []byte + expectError bool + expectOverlappedEntries []string + expectedConfig *credentialProviderConfigWithVersion + }{ + { + name: "add crio-credential-provider when template config is nil", + matchImages: []string{"myhost.com", "quay.io"}, + templateConfig: nil, + expectedConfig: &credentialProviderConfigWithVersion{ + APIVersion: "kubelet.config.k8s.io/v1", + Kind: "CredentialProviderConfig", + Providers: []*credentialProviderWithTag{ + { + Name: "crio-credential-provider", + MatchImages: []string{"myhost.com", "quay.io"}, + APIVersion: "credentialprovider.kubelet.k8s.io/v1", + DefaultCacheDuration: &metav1.Duration{Duration: time.Second}, + TokenAttributes: &serviceAccountTokenAttributesVersioned{ + ServiceAccountTokenAudience: "https://kubernetes.default.svc", + CacheType: "Token", + RequireServiceAccount: ptr.To(false), + }, + }, + }, + }, + }, + { + name: "add crio-credential-provider when not present", + matchImages: []string{"myhost.com", "quay.io"}, + templateConfig: templateCredProviderConfig, + expectedConfig: &credentialProviderConfigWithVersion{ + APIVersion: "kubelet.config.k8s.io/v1", + Kind: "CredentialProviderConfig", + Providers: []*credentialProviderWithTag{ + { + Name: "gcr-credential-provider", + APIVersion: "credentialprovider.kubelet.k8s.io/v1", + MatchImages: []string{"gcr.io", "*.gcr.io", "*.pkg.dev", "container.cloud.google.com"}, + DefaultCacheDuration: &metav1.Duration{Duration: time.Minute}, + Args: []string{"get-credentials", "--v=3"}, + }, + { + Name: "crio-credential-provider", + MatchImages: []string{"myhost.com", "quay.io"}, + APIVersion: "credentialprovider.kubelet.k8s.io/v1", + DefaultCacheDuration: &metav1.Duration{Duration: time.Second}, + TokenAttributes: &serviceAccountTokenAttributesVersioned{ + ServiceAccountTokenAudience: "https://kubernetes.default.svc", + CacheType: "Token", + RequireServiceAccount: ptr.To(false), + }, + }, + }, + }, + }, + { + name: "crio-credential-provider already present", + matchImages: []string{"myhost.com"}, + templateConfig: templateCredProviderConfigWithCRIOProvider, + expectedConfig: &credentialProviderConfigWithVersion{ + APIVersion: "kubelet.config.k8s.io/v1", + Kind: "CredentialProviderConfig", + Providers: []*credentialProviderWithTag{ + { + Name: "gcr-credential-provider", + APIVersion: "credentialprovider.kubelet.k8s.io/v1", + MatchImages: []string{"gcr.io", "*.gcr.io", "*.pkg.dev", "container.cloud.google.com"}, + DefaultCacheDuration: &metav1.Duration{Duration: time.Minute}, + Args: []string{"get-credentials", "--v=3"}, + }, + { + Name: "crio-credential-provider", + MatchImages: []string{"myhost.com"}, + APIVersion: "credentialprovider.kubelet.k8s.io/v1", + DefaultCacheDuration: &metav1.Duration{Duration: time.Second}, + TokenAttributes: &serviceAccountTokenAttributesVersioned{ + ServiceAccountTokenAudience: "https://kubernetes.default.svc", + CacheType: "Token", + RequireServiceAccount: ptr.To(false), + }, + }, + }, + }, + }, + + { + name: "all matchImages overlap with existing provider - returns nil", + matchImages: []string{"gcr.io", "*.cloud.google.com", "*.gcr.io", "*.pkg.dev"}, + templateConfig: templateCredProviderConfig, + expectOverlappedEntries: []string{ + "*.cloud.google.com", + "*.gcr.io", + "*.pkg.dev", + "gcr.io", + }, + // When all matchImages overlap, updatedConfigBytes is nil + // This is equivalent to not calling updateCredentialProviderConfig at all + expectedConfig: nil, + }, + { + name: "overlapping matchImages with existing provider", + matchImages: []string{"gcr.io", "myhost.com", "*.cloud.google.com", "quay.io"}, + templateConfig: templateCredProviderConfig, + expectOverlappedEntries: []string{ + "*.cloud.google.com", + "gcr.io", + }, + expectedConfig: &credentialProviderConfigWithVersion{ + APIVersion: "kubelet.config.k8s.io/v1", + Kind: "CredentialProviderConfig", + Providers: []*credentialProviderWithTag{ + { + Name: "gcr-credential-provider", + APIVersion: "credentialprovider.kubelet.k8s.io/v1", + MatchImages: []string{"gcr.io", "*.gcr.io", "*.pkg.dev", "container.cloud.google.com"}, + DefaultCacheDuration: &metav1.Duration{Duration: time.Minute}, + Args: []string{"get-credentials", "--v=3"}, + }, + { + Name: "crio-credential-provider", + MatchImages: []string{"myhost.com", "quay.io"}, + APIVersion: "credentialprovider.kubelet.k8s.io/v1", + DefaultCacheDuration: &metav1.Duration{Duration: time.Second}, + TokenAttributes: &serviceAccountTokenAttributesVersioned{ + ServiceAccountTokenAudience: "https://kubernetes.default.svc", + CacheType: "Token", + RequireServiceAccount: ptr.To(false), + }, + }, + }, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + + var testImages []apicfgv1alpha1.MatchImage + for _, img := range tt.matchImages { + testImages = append(testImages, apicfgv1alpha1.MatchImage(img)) + } + + credProviderConfigObject, err := credProviderConfigObject(tt.templateConfig) + require.NoError(t, err) + require.NotNil(t, credProviderConfigObject) + updatedConfigBytes, overlappedEntries, err := updateCredentialProviderConfig(credProviderConfigObject, testImages) + if tt.expectError { + require.Error(t, err) + return + } + require.NoError(t, err) + if len(tt.expectOverlappedEntries) > 0 { + require.Equal(t, tt.expectOverlappedEntries, overlappedEntries, "expected overlapped entries") + } else { + require.Empty(t, overlappedEntries, "expected no overlapped entries") + } + + if tt.expectedConfig == nil { + // When all matchImages overlap, updatedConfigBytes should be nil + require.Nil(t, updatedConfigBytes, "expected nil config when all matchImages overlap") + } else { + var gotConfig credentialProviderConfigWithVersion + err = yaml.Unmarshal(updatedConfigBytes, &gotConfig) + require.NoError(t, err) + assert.Equal(t, tt.expectedConfig, &gotConfig) + } + }) + } + +} diff --git a/pkg/daemon/constants/constants.go b/pkg/daemon/constants/constants.go index fbfe1b5e01..677c2cad97 100644 --- a/pkg/daemon/constants/constants.go +++ b/pkg/daemon/constants/constants.go @@ -145,6 +145,10 @@ const ( // changes to this path. Note that other files added to the parent directory will not be handled specially GPGNoRebootPath = "/etc/machine-config-daemon/no-reboot/containers-gpg.pub" + KubernetesCredentialProvidersDir = "/etc/kubernetes/credential-providers" + + KubeletCrioImageCredProviderConfPath = "/etc/systemd/system/kubelet.service.d/40-kubelet-crio-image-credential-provider.conf" + // rpm-ostree command arguments RPMOSTreeUpdateArg = "update" RPMOSTreeInstallArg = "--install" diff --git a/templates/arbiter/01-arbiter-kubelet/_base/units/kubelet.service.yaml b/templates/arbiter/01-arbiter-kubelet/_base/units/kubelet.service.yaml index 1583c6c104..4b3ca4383f 100644 --- a/templates/arbiter/01-arbiter-kubelet/_base/units/kubelet.service.yaml +++ b/templates/arbiter/01-arbiter-kubelet/_base/units/kubelet.service.yaml @@ -39,7 +39,7 @@ contents: | --minimum-container-ttl-duration=6m0s \ --cloud-provider={{cloudProvider .}} \ --volume-plugin-dir=/etc/kubernetes/kubelet-plugins/volume/exec \ - {{credentialProviderConfigFlag . }} \ + {{credentialProviderConfigFlag . }} $KUBELET_CRIO_IMAGE_CREDENTIAL_PROVIDER_FLAGS \ --hostname-override=${KUBELET_NODE_NAME} \ --provider-id=${KUBELET_PROVIDERID} \ --register-with-taints=node-role.kubernetes.io/arbiter=:NoSchedule \ diff --git a/templates/arbiter/01-arbiter-kubelet/on-prem/units/kubelet.service.yaml b/templates/arbiter/01-arbiter-kubelet/on-prem/units/kubelet.service.yaml index 63002f660c..a5c119e073 100644 --- a/templates/arbiter/01-arbiter-kubelet/on-prem/units/kubelet.service.yaml +++ b/templates/arbiter/01-arbiter-kubelet/on-prem/units/kubelet.service.yaml @@ -38,6 +38,7 @@ contents: | --minimum-container-ttl-duration=6m0s \ --cloud-provider={{cloudProvider .}} \ --volume-plugin-dir=/etc/kubernetes/kubelet-plugins/volume/exec \ + $KUBELET_CRIO_IMAGE_CREDENTIAL_PROVIDER_FLAGS \ --hostname-override=${KUBELET_NODE_NAME} \ --register-with-taints=node-role.kubernetes.io/master=:NoSchedule,node-role.kubernetes.io/arbiter=:NoSchedule \ --system-reserved=cpu=${SYSTEM_RESERVED_CPU},memory=${SYSTEM_RESERVED_MEMORY},ephemeral-storage=${SYSTEM_RESERVED_ES} \ diff --git a/templates/master/01-master-kubelet/on-prem/units/kubelet.service.yaml b/templates/master/01-master-kubelet/on-prem/units/kubelet.service.yaml index c203faa4c9..75d46409b9 100644 --- a/templates/master/01-master-kubelet/on-prem/units/kubelet.service.yaml +++ b/templates/master/01-master-kubelet/on-prem/units/kubelet.service.yaml @@ -38,6 +38,7 @@ contents: | --minimum-container-ttl-duration=6m0s \ --cloud-provider={{cloudProvider .}} \ --volume-plugin-dir=/etc/kubernetes/kubelet-plugins/volume/exec \ + $KUBELET_CRIO_IMAGE_CREDENTIAL_PROVIDER_FLAGS \ --hostname-override=${KUBELET_NODE_NAME} \ --register-with-taints=node-role.kubernetes.io/master=:NoSchedule \ --system-reserved=cpu=${SYSTEM_RESERVED_CPU},memory=${SYSTEM_RESERVED_MEMORY},ephemeral-storage=${SYSTEM_RESERVED_ES} \ diff --git a/templates/worker/01-worker-kubelet/on-prem/units/kubelet.service.yaml b/templates/worker/01-worker-kubelet/on-prem/units/kubelet.service.yaml index 0aa5798710..70729db986 100644 --- a/templates/worker/01-worker-kubelet/on-prem/units/kubelet.service.yaml +++ b/templates/worker/01-worker-kubelet/on-prem/units/kubelet.service.yaml @@ -39,6 +39,7 @@ contents: | --address=${KUBELET_NODE_IP} \ --minimum-container-ttl-duration=6m0s \ --volume-plugin-dir=/etc/kubernetes/kubelet-plugins/volume/exec \ + $KUBELET_CRIO_IMAGE_CREDENTIAL_PROVIDER_FLAGS \ --cloud-provider={{cloudProvider .}} \ --hostname-override=${KUBELET_NODE_NAME} \ --system-reserved=cpu=${SYSTEM_RESERVED_CPU},memory=${SYSTEM_RESERVED_MEMORY},ephemeral-storage=${SYSTEM_RESERVED_ES} \ diff --git a/test/framework/envtest.go b/test/framework/envtest.go index 3fb1a5863a..a3e99185ab 100644 --- a/test/framework/envtest.go +++ b/test/framework/envtest.go @@ -116,6 +116,9 @@ func NewTestEnv(t *testing.T) *envtest.Environment { filepath.Join("..", "..", "vendor", "github.com", "openshift", "api", "operator", "v1", "zz_generated.crd-manifests"), filepath.Join("..", "..", "vendor", "github.com", "openshift", "api", "operator", "v1alpha1", "zz_generated.crd-manifests"), filepath.Join("..", "..", "vendor", "github.com", "openshift", "api", "config", "v1", "zz_generated.crd-manifests"), + filepath.Join("..", "..", "vendor", "github.com", "openshift", "api", "config", "v1alpha1", "zz_generated.crd-manifests", "0000_10_config-operator_01_criocredentialproviderconfigs-CustomNoUpgrade.crd.yaml"), + filepath.Join("..", "..", "vendor", "github.com", "openshift", "api", "config", "v1alpha1", "zz_generated.crd-manifests", "0000_10_config-operator_01_criocredentialproviderconfigs-DevPreviewNoUpgrade.crd.yaml"), + filepath.Join("..", "..", "vendor", "github.com", "openshift", "api", "config", "v1alpha1", "zz_generated.crd-manifests", "0000_10_config-operator_01_criocredentialproviderconfigs-TechPreviewNoUpgrade.crd.yaml"), filepath.Join("..", "..", "vendor", "github.com", "openshift", "api", "machineconfiguration", "v1", "zz_generated.crd-manifests"), filepath.Join("..", "..", "vendor", "github.com", "openshift", "api", "machineconfiguration", "v1alpha1", "zz_generated.crd-manifests"), },