Skip to content

Commit

Permalink
Switch traceflow CRD validation to webhook validation.
Browse files Browse the repository at this point in the history
Currently, the traceflow CRD validation is executed in run-time, which is less
user-friendly than the webhook validation. I moved most of the validation to
the webhook validation.

Signed-off-by: shi0rik0 <anguuan@outlook.com>
  • Loading branch information
shi0rik0 committed Jul 12, 2023
1 parent 1174935 commit fa1c2d3
Show file tree
Hide file tree
Showing 12 changed files with 370 additions and 36 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -141,3 +141,18 @@ webhooks:
admissionReviewVersions: ["v1", "v1beta1"]
sideEffects: None
timeoutSeconds: 5
- name: "traceflowvalidator.antrea.io"
clientConfig:
service:
name: "antrea"
namespace: {{ .Release.Namespace }}
path: "/validate/traceflow"
rules:
- operations: ["CREATE", "UPDATE"]
apiGroups: ["crd.antrea.io"]
apiVersions: ["v1alpha1"]
resources: ["traceflows"]
scope: "Cluster"
admissionReviewVersions: ["v1", "v1beta1"]
sideEffects: None
timeoutSeconds: 5
15 changes: 15 additions & 0 deletions build/yamls/antrea-aks.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5004,3 +5004,18 @@ webhooks:
admissionReviewVersions: ["v1", "v1beta1"]
sideEffects: None
timeoutSeconds: 5
- name: "traceflowvalidator.antrea.io"
clientConfig:
service:
name: "antrea"
namespace: kube-system
path: "/validate/traceflow"
rules:
- operations: ["CREATE", "UPDATE"]
apiGroups: ["crd.antrea.io"]
apiVersions: ["v1alpha1"]
resources: ["traceflows"]
scope: "Cluster"
admissionReviewVersions: ["v1", "v1beta1"]
sideEffects: None
timeoutSeconds: 5
15 changes: 15 additions & 0 deletions build/yamls/antrea-eks.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5005,3 +5005,18 @@ webhooks:
admissionReviewVersions: ["v1", "v1beta1"]
sideEffects: None
timeoutSeconds: 5
- name: "traceflowvalidator.antrea.io"
clientConfig:
service:
name: "antrea"
namespace: kube-system
path: "/validate/traceflow"
rules:
- operations: ["CREATE", "UPDATE"]
apiGroups: ["crd.antrea.io"]
apiVersions: ["v1alpha1"]
resources: ["traceflows"]
scope: "Cluster"
admissionReviewVersions: ["v1", "v1beta1"]
sideEffects: None
timeoutSeconds: 5
15 changes: 15 additions & 0 deletions build/yamls/antrea-gke.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5002,3 +5002,18 @@ webhooks:
admissionReviewVersions: ["v1", "v1beta1"]
sideEffects: None
timeoutSeconds: 5
- name: "traceflowvalidator.antrea.io"
clientConfig:
service:
name: "antrea"
namespace: kube-system
path: "/validate/traceflow"
rules:
- operations: ["CREATE", "UPDATE"]
apiGroups: ["crd.antrea.io"]
apiVersions: ["v1alpha1"]
resources: ["traceflows"]
scope: "Cluster"
admissionReviewVersions: ["v1", "v1beta1"]
sideEffects: None
timeoutSeconds: 5
15 changes: 15 additions & 0 deletions build/yamls/antrea-ipsec.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5061,3 +5061,18 @@ webhooks:
admissionReviewVersions: ["v1", "v1beta1"]
sideEffects: None
timeoutSeconds: 5
- name: "traceflowvalidator.antrea.io"
clientConfig:
service:
name: "antrea"
namespace: kube-system
path: "/validate/traceflow"
rules:
- operations: ["CREATE", "UPDATE"]
apiGroups: ["crd.antrea.io"]
apiVersions: ["v1alpha1"]
resources: ["traceflows"]
scope: "Cluster"
admissionReviewVersions: ["v1", "v1beta1"]
sideEffects: None
timeoutSeconds: 5
15 changes: 15 additions & 0 deletions build/yamls/antrea.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5002,3 +5002,18 @@ webhooks:
admissionReviewVersions: ["v1", "v1beta1"]
sideEffects: None
timeoutSeconds: 5
- name: "traceflowvalidator.antrea.io"
clientConfig:
service:
name: "antrea"
namespace: kube-system
path: "/validate/traceflow"
rules:
- operations: ["CREATE", "UPDATE"]
apiGroups: ["crd.antrea.io"]
apiVersions: ["v1alpha1"]
resources: ["traceflows"]
scope: "Cluster"
admissionReviewVersions: ["v1", "v1beta1"]
sideEffects: None
timeoutSeconds: 5
6 changes: 5 additions & 1 deletion cmd/antrea-controller/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@ var allowedPaths = []string{
"/validate/group",
"/validate/ippool",
"/validate/supportbundlecollection",
"/validate/traceflow",
"/convert/clustergroup",
}

Expand Down Expand Up @@ -291,6 +292,7 @@ func run(o *Options) error {
egressController,
statsAggregator,
bundleCollectionController,
traceflowController,
*o.config.EnablePrometheusMetrics,
cipherSuites,
cipher.TLSVersionMap[o.config.TLSMinVersion])
Expand Down Expand Up @@ -476,6 +478,7 @@ func createAPIServerConfig(kubeconfig string,
egressController *egress.EgressController,
statsAggregator *stats.Aggregator,
bundleCollectionStore *supportbundlecollection.Controller,
traceflowController *traceflow.Controller,
enableMetrics bool,
cipherSuites []uint16,
tlsMinVersion uint16) (*apiserver.Config, error) {
Expand Down Expand Up @@ -542,5 +545,6 @@ func createAPIServerConfig(kubeconfig string,
endpointQuerier,
npController,
egressController,
bundleCollectionStore), nil
bundleCollectionStore,
traceflowController), nil
}
14 changes: 1 addition & 13 deletions pkg/agent/controller/traceflow/traceflow_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -300,16 +300,6 @@ func (c *Controller) startTraceflow(tf *crdv1alpha1.Traceflow) error {
return err
}

liveTraffic := tf.Spec.LiveTraffic
if tf.Spec.Source.Pod == "" && tf.Spec.Destination.Pod == "" {
klog.Errorf("Traceflow %s has neither source nor destination Pod specified", tf.Name)
return nil
}
if tf.Spec.Source.Pod == "" && !liveTraffic {
klog.Errorf("Traceflow %s does not have source Pod specified", tf.Name)
return nil
}

receiverOnly := false
var pod, ns string
if tf.Spec.Source.Pod != "" {
Expand All @@ -327,6 +317,7 @@ func (c *Controller) startTraceflow(tf *crdv1alpha1.Traceflow) error {
podInterfaces := c.interfaceStore.GetContainerInterfacesByPod(pod, ns)
isSender := len(podInterfaces) > 0 && !receiverOnly

liveTraffic := tf.Spec.LiveTraffic
var packet, matchPacket *binding.Packet
var ofPort uint32
if len(podInterfaces) > 0 {
Expand Down Expand Up @@ -388,9 +379,6 @@ func (c *Controller) validateTraceflow(tf *crdv1alpha1.Traceflow) error {
}
if tf.Spec.Destination.IP != "" {
destIP := net.ParseIP(tf.Spec.Destination.IP)
if destIP == nil {
return fmt.Errorf("destination IP is not valid: %s", tf.Spec.Destination.IP)
}
// When AntreaProxy is enabled, serviceCIDR is not required and may be set to a
// default value which does not match the cluster configuration.
if !features.DefaultFeatureGate.Enabled(features.AntreaProxy) && c.serviceCIDR.Contains(destIP) {
Expand Down
10 changes: 9 additions & 1 deletion pkg/apiserver/apiserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ import (
"antrea.io/antrea/pkg/controller/querier"
"antrea.io/antrea/pkg/controller/stats"
controllerbundlecollection "antrea.io/antrea/pkg/controller/supportbundlecollection"
"antrea.io/antrea/pkg/controller/traceflow"
"antrea.io/antrea/pkg/features"
)

Expand Down Expand Up @@ -116,6 +117,7 @@ type ExtraConfig struct {
statsAggregator *stats.Aggregator
networkPolicyStatusController *controllernetworkpolicy.StatusController
bundleCollectionController *controllerbundlecollection.Controller
traceflowController *traceflow.Controller
}

// Config defines the config for Antrea apiserver.
Expand Down Expand Up @@ -158,7 +160,8 @@ func NewConfig(
endpointQuerier controllernetworkpolicy.EndpointQuerier,
npController *controllernetworkpolicy.NetworkPolicyController,
egressController *egress.EgressController,
bundleCollectionController *controllerbundlecollection.Controller) *Config {
bundleCollectionController *controllerbundlecollection.Controller,
traceflowController *traceflow.Controller) *Config {
return &Config{
genericConfig: genericConfig,
extraConfig: ExtraConfig{
Expand All @@ -178,6 +181,7 @@ func NewConfig(
networkPolicyStatusController: networkPolicyStatusController,
egressController: egressController,
bundleCollectionController: bundleCollectionController,
traceflowController: traceflowController,
},
}
}
Expand Down Expand Up @@ -333,6 +337,10 @@ func installHandlers(c *ExtraConfig, s *genericapiserver.GenericAPIServer) {
if features.DefaultFeatureGate.Enabled(features.SupportBundleCollection) {
s.Handler.NonGoRestfulMux.HandleFunc("/validate/supportbundlecollection", webhook.HandlerForValidateFunc(c.bundleCollectionController.Validate))
}

if features.DefaultFeatureGate.Enabled(features.Traceflow) {
s.Handler.NonGoRestfulMux.HandleFunc("/validate/traceflow", webhook.HandlerForValidateFunc(c.traceflowController.Validate))
}
}

func DefaultCAConfig() *certificate.CAConfig {
Expand Down
21 changes: 0 additions & 21 deletions pkg/controller/traceflow/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ import (
crdinformers "antrea.io/antrea/pkg/client/informers/externalversions/crd/v1alpha1"
crdlisters "antrea.io/antrea/pkg/client/listers/crd/v1alpha1"
"antrea.io/antrea/pkg/controller/grouping"
"antrea.io/antrea/pkg/util/k8s"
)

const (
Expand Down Expand Up @@ -249,10 +248,6 @@ func (c *Controller) syncTraceflow(traceflowName string) error {
}

func (c *Controller) startTraceflow(tf *crdv1alpha1.Traceflow) error {
if err := c.validateTraceflow(tf); err != nil {
klog.ErrorS(err, "Invalid Traceflow request", "request", tf)
return c.updateTraceflowStatus(tf, crdv1alpha1.Failed, fmt.Sprintf("Invalid Traceflow request, err: %+v", err), 0)
}
// Allocate data plane tag.
tag, err := c.allocateTag(tf.Name)
if err != nil {
Expand Down Expand Up @@ -413,19 +408,3 @@ func (c *Controller) deallocateTag(name string, tag uint8) {
}
}
}

func (c *Controller) validateTraceflow(tf *crdv1alpha1.Traceflow) error {
if !tf.Spec.LiveTraffic {
srcPod, err := c.podLister.Pods(tf.Spec.Source.Namespace).Get(tf.Spec.Source.Pod)
if err != nil {
if apierrors.IsNotFound(err) {
err = fmt.Errorf("requested source Pod %s not found", k8s.NamespacedName(tf.Spec.Source.Namespace, tf.Spec.Source.Pod))
}
return err
}
if srcPod.Spec.HostNetwork {
return fmt.Errorf("using hostNetwork Pod as source in non-live-traffic Traceflow is not supported")
}
}
return nil
}
104 changes: 104 additions & 0 deletions pkg/controller/traceflow/validate.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
// Copyright 2023 Antrea Authors
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package traceflow

import (
"encoding/json"
"fmt"
"net"

admv1 "k8s.io/api/admission/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/klog/v2"

crdv1alpha1 "antrea.io/antrea/pkg/apis/crd/v1alpha1"
"antrea.io/antrea/pkg/util/k8s"
)

func (c *Controller) Validate(review *admv1.AdmissionReview) *admv1.AdmissionResponse {
newResponse := func(allowed bool, deniedReason string) *admv1.AdmissionResponse {
resp := &admv1.AdmissionResponse{
UID: review.Request.UID,
Allowed: allowed,
}
if !allowed {
resp.Result = &metav1.Status{
Message: deniedReason,
}
}
return resp
}

klog.V(2).InfoS("Validating Traceflow", "request", review.Request)

var newObj, oldObj crdv1alpha1.Traceflow
if review.Request.Object.Raw != nil {
if err := json.Unmarshal(review.Request.Object.Raw, &newObj); err != nil {
klog.ErrorS(err, "Error de-serializing current Traceflow")
return newResponse(false, err.Error())
}
}
if review.Request.OldObject.Raw != nil {
if err := json.Unmarshal(review.Request.OldObject.Raw, &oldObj); err != nil {
klog.ErrorS(err, "Error de-serializing old Traceflow")
return newResponse(false, err.Error())
}
}

switch review.Request.Operation {
case admv1.Create:
klog.V(2).InfoS("Validating CREATE request for Traceflow")
allowed, deniedReason := c.validateCreate(&newObj)
return newResponse(allowed, deniedReason)
case admv1.Update:
klog.V(2).InfoS("Validating UPDATE request for Traceflow")
allowed, deniedReason := c.validateUpdate(&oldObj, &newObj)
return newResponse(allowed, deniedReason)
default:
err := fmt.Errorf("invalid request operation %s for Traceflow", review.Request.Operation)
klog.ErrorS(err, "Failed to validate Traceflow")
return newResponse(false, err.Error())
}
}

func (c *Controller) validateCreate(tf *crdv1alpha1.Traceflow) (allowed bool, deniedReason string) {
if tf.Spec.Destination.IP != "" {
destIP := net.ParseIP(tf.Spec.Destination.IP)
if destIP == nil {
return false, fmt.Sprintf("destination IP is not valid: %s", tf.Spec.Destination.IP)
}
}
if !tf.Spec.LiveTraffic {
srcPod, err := c.podLister.Pods(tf.Spec.Source.Namespace).Get(tf.Spec.Source.Pod)
if err != nil {
if apierrors.IsNotFound(err) {
err = fmt.Errorf("requested source Pod %s not found", k8s.NamespacedName(tf.Spec.Source.Namespace, tf.Spec.Source.Pod))
}
return false, err.Error()
}
if srcPod.Spec.HostNetwork {
return false, "using hostNetwork Pod as source in non-live-traffic Traceflow is not supported"
}
}
if tf.Spec.Source.Pod == "" && tf.Spec.Destination.Pod == "" {
return false, fmt.Sprintf("Traceflow %s has neither source nor destination Pod specified", tf.Name)
}
return true, ""
}

func (c *Controller) validateUpdate(oldTf, newTf *crdv1alpha1.Traceflow) (allowed bool, deniedReason string) {
return c.validateCreate(newTf)
}
Loading

0 comments on commit fa1c2d3

Please sign in to comment.