diff --git a/CHANGELOG.md b/CHANGELOG.md index f718ee7100444..ece464439e5e3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ Please avoid adding duplicate information across this changelog and JIRA/doc inp ## [4.7.2] ### Added Features +- ROX-13493: Support for scale subresource in the admission controller to enable policy detection and enforcement on admission review requests on the scale subresource. - ROX-28716: New policy criterion "Days Since CVE Was Published" to allow creation of a policy that offers a grace period to teams to fix vulnerabilities within the number of days from when the CVE was published in the vulnerability feeds. diff --git a/sensor/admission-control/manager/evaluate_deploytime.go b/sensor/admission-control/manager/evaluate_deploytime.go index ccfdf2dace5f1..33ca927712bee 100644 --- a/sensor/admission-control/manager/evaluate_deploytime.go +++ b/sensor/admission-control/manager/evaluate_deploytime.go @@ -19,6 +19,10 @@ import ( "k8s.io/utils/pointer" ) +const ( + ScaleSubResource = "scale" +) + var ( detectionCtx = deploytime.DetectionContext{ EnforcementOnly: true, @@ -37,9 +41,13 @@ func (m *manager) shouldBypass(s *state, req *admission.AdmissionRequest) bool { return true } - // We don't enforce on subresources. - if req.SubResource != "" { - log.Debugf("Request is for a subresource, bypassing %s request on %s/%s [%s]", req.Operation, req.Namespace, req.Name, req.Kind) + // We don't enforce on subresources other than the scale subresource. + // Openshift console uses the scale subresource to scale deployments, and our admission controller bypasses these requests + // without running policy detection and enforcement. However, an `oc scale` command works. The following + // change makes the behavior of admission controller consistent across all supported ways that k8s allows + // deployment replica scaling. + if req.SubResource != "" && req.SubResource != ScaleSubResource { + log.Debugf("Request is for a subresource other than the scale subresource, bypassing %s request on %s/%s [%s]", req.Operation, req.Namespace, req.Name, req.Kind) return true } @@ -102,21 +110,29 @@ func (m *manager) evaluateAdmissionRequest(s *state, req *admission.AdmissionReq log.Debugf("Not bypassing %s request on %s/%s [%s]", req.Operation, req.Namespace, req.Name, req.Kind) - k8sObj, err := unmarshalK8sObject(req.Kind, req.Object.Raw) - if err != nil { - return nil, errors.Wrap(err, "could not unmarshal object from request") - } + var deployment *storage.Deployment + if req.SubResource != "" && req.SubResource == ScaleSubResource { + if deployment = m.deployments.GetByName(req.Namespace, req.Name); deployment == nil { + return nil, errors.Errorf( + "could not find deployment with name: %q in namespace %q for this admission review request", + req.Name, req.Namespace) + } + } else { + k8sObj, err := unmarshalK8sObject(req.Kind, req.Object.Raw) + if err != nil { + return nil, errors.Wrap(err, "could not unmarshal object from request") + } - deployment, err := resources.NewDeploymentFromStaticResource(k8sObj, req.Kind.Kind, s.clusterID(), s.GetClusterConfig().GetRegistryOverride()) - if err != nil { - return nil, errors.Wrap(err, "could not convert Kubernetes object into StackRox deployment") - } + deployment, err = resources.NewDeploymentFromStaticResource(k8sObj, req.Kind.Kind, s.clusterID(), s.GetClusterConfig().GetRegistryOverride()) + if err != nil { + return nil, errors.Wrap(err, "could not convert Kubernetes object into StackRox deployment") + } - if deployment == nil { - log.Debugf("Non-top-level object, bypassing %s request on %s/%s [%s]", req.Operation, req.Namespace, req.Name, req.Kind) - return pass(req.UID), nil // we only enforce on top-level objects + if deployment == nil { + log.Debugf("Non-top-level object, bypassing %s request on %s/%s [%s]", req.Operation, req.Namespace, req.Name, req.Kind) + return pass(req.UID), nil // we only enforce on top-level objects + } } - log.Debugf("Evaluating policies on %+v", deployment) // Check if the deployment has a bypass annotation diff --git a/sensor/admission-control/manager/images.go b/sensor/admission-control/manager/images.go index 2f70ed7c01cb3..a2af7602080d0 100644 --- a/sensor/admission-control/manager/images.go +++ b/sensor/admission-control/manager/images.go @@ -153,6 +153,14 @@ func hasModifiedImages(s *state, deployment *storage.Deployment, req *admission. return true } + if req.SubResource != "" && req.SubResource == ScaleSubResource { + // TODO: We could consider returning false here since when the admission review request is for the scale + // subresource, I do not believe it is possible for a user to change the image on the deployment at the same + // time as updating the scale subresource However, the contract of this function as designed was to be + // conservative and return true. + return true + } + oldK8sObj, err := unmarshalK8sObject(req.Kind, req.OldObject.Raw) if err != nil { log.Errorf("Failed to unmarshal old object into K8s object: %v", err) @@ -164,7 +172,6 @@ func hasModifiedImages(s *state, deployment *storage.Deployment, req *admission. log.Errorf("Failed to convert old K8s object into StackRox deployment: %v", err) return true } - if oldDeployment == nil { return true } diff --git a/sensor/admission-control/manager/responses.go b/sensor/admission-control/manager/responses.go index c3d68ab2d78be..3a2ad79ad50dd 100644 --- a/sensor/admission-control/manager/responses.go +++ b/sensor/admission-control/manager/responses.go @@ -5,6 +5,7 @@ import ( "text/template" "github.com/stackrox/rox/generated/storage" + "github.com/stackrox/rox/pkg/branding" "github.com/stackrox/rox/pkg/enforcers" "github.com/stackrox/rox/pkg/stringutils" "github.com/stackrox/rox/pkg/templates" @@ -55,7 +56,7 @@ func fail(uid types.UID, message string) *admission.AdmissionResponse { Allowed: false, Result: &metav1.Status{ Status: "Failure", - Reason: metav1.StatusReason("Failed currently enforced policies from StackRox"), + Reason: metav1.StatusReason(fmt.Sprintf("Failed currently enforced policies from %s", branding.GetProductNameShort())), Message: message, }, } diff --git a/sensor/admission-control/resources/deployment.go b/sensor/admission-control/resources/deployment.go index bd22311acfbfd..15b6751fa35ba 100644 --- a/sensor/admission-control/resources/deployment.go +++ b/sensor/admission-control/resources/deployment.go @@ -9,15 +9,20 @@ import ( // NewDeploymentStore returns new instance of DeploymentStore. func NewDeploymentStore(pods *PodStore) *DeploymentStore { return &DeploymentStore{ - deployments: make(map[string]map[string]*storage.Deployment), - pods: pods, + deployments: make(map[string]map[string]*storage.Deployment), + deploymentNamesToIds: make(map[string]map[string]string), + pods: pods, } } // DeploymentStore stores the deployments. type DeploymentStore struct { + // A map of maps of deployment ID -> deployment object, by namespace deployments map[string]map[string]*storage.Deployment - pods *PodStore + // A map of maps of deployment names to IDs, by namespace. + deploymentNamesToIds map[string]map[string]string + + pods *PodStore mutex sync.RWMutex } @@ -35,14 +40,23 @@ func (m *DeploymentStore) ProcessEvent(action central.ResourceAction, obj interf switch action { case central.ResourceAction_CREATE_RESOURCE, central.ResourceAction_UPDATE_RESOURCE, central.ResourceAction_SYNC_RESOURCE: depMap := m.deployments[deployment.GetNamespace()] + depNameToIdMap := m.deploymentNamesToIds[deployment.GetNamespace()] if depMap == nil { depMap = make(map[string]*storage.Deployment) m.deployments[deployment.GetNamespace()] = depMap } depMap[deployment.GetId()] = deployment + + if depNameToIdMap == nil { + depNameToIdMap = make(map[string]string) + m.deploymentNamesToIds[deployment.GetNamespace()] = depNameToIdMap + } + depNameToIdMap[deployment.GetName()] = deployment.GetId() + case central.ResourceAction_REMOVE_RESOURCE: // Deployment remove event contains full deployment object. delete(m.deployments[deployment.GetNamespace()], deployment.GetId()) + delete(m.deploymentNamesToIds[deployment.GetNamespace()], deployment.GetName()) m.pods.OnDeploymentDelete(deployment.GetNamespace(), deployment.GetId()) } } @@ -59,10 +73,30 @@ func (m *DeploymentStore) Get(namespace, deploymentID string) *storage.Deploymen return depMap[deploymentID] } +// GetByName returns a deployment given namespace and deployment name. +func (m *DeploymentStore) GetByName(namespace, name string) *storage.Deployment { + m.mutex.RLock() + defer m.mutex.RUnlock() + + deploymentNamesToIdsMap := m.deploymentNamesToIds[namespace] + if deploymentNamesToIdsMap == nil { + return nil + } + + depMap := m.deployments[namespace] + if depMap == nil { + return nil + } + + deploymentID := deploymentNamesToIdsMap[name] + return depMap[deploymentID] +} + // OnNamespaceDelete removes deployments in supplied namespace. func (m *DeploymentStore) OnNamespaceDelete(namespace string) { m.mutex.Lock() defer m.mutex.Unlock() delete(m.deployments, namespace) + delete(m.deploymentNamesToIds, namespace) }