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
110 changes: 38 additions & 72 deletions pkg/controller/applicationlayer/applicationlayer_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"github.com/tigera/operator/pkg/common"
"github.com/tigera/operator/pkg/controller/gatewayapi"
"github.com/tigera/operator/pkg/controller/options"
"github.com/tigera/operator/pkg/controller/sharedconfig"
"github.com/tigera/operator/pkg/controller/status"
"github.com/tigera/operator/pkg/controller/utils"
"github.com/tigera/operator/pkg/controller/utils/imageset"
Expand Down Expand Up @@ -80,6 +81,7 @@ func newReconciler(mgr manager.Manager, opts options.ControllerOptions, licenseA
provider: opts.DetectedProvider,
status: status.New(mgr.GetClient(), "applicationlayer", opts.KubernetesVersion),
clusterDomain: opts.ClusterDomain,
useV3CRDs: opts.UseV3CRDs,
variant: opts.Variant,
licenseAPIReady: licenseAPIReady,
}
Expand Down Expand Up @@ -166,6 +168,7 @@ type ReconcileApplicationLayer struct {
provider operatorv1.Provider
status status.StatusManager
clusterDomain string
useV3CRDs bool
variant operatorv1.ProductVariant
licenseAPIReady *utils.ReadyFlag
}
Expand Down Expand Up @@ -486,11 +489,6 @@ func (r *ReconcileApplicationLayer) isSidecarInjectionEnabled(applicationLayerSp
*applicationLayerSpec.SidecarInjection == operatorv1.SidecarEnabled
}

func (r *ReconcileApplicationLayer) getPolicySyncPathPrefix(fcSpec *v3.FelixConfigurationSpec, al *operatorv1.ApplicationLayer, istioNeeds bool) string {
alNeeds := utils.ApplicationLayerRequiresPolicySync(al)
return utils.DesiredPolicySyncPathPrefix(fcSpec.PolicySyncPathPrefix, alNeeds, istioNeeds)
}

func (r *ReconcileApplicationLayer) getTProxyMode(al *operatorv1.ApplicationLayer) (bool, string) {
if al == nil {
return false, "Disabled"
Expand All @@ -507,75 +505,43 @@ func (r *ReconcileApplicationLayer) getTProxyMode(al *operatorv1.ApplicationLaye
return true, "Disabled"
}

// patchFelixConfiguration takes all application layer specs as arguments and patches felix config.
// If at least one of the specs requires TPROXYMode as "Enabled" it'll be patched as "Enabled" otherwise it is "Disabled".
// gatewayWAFEnabled reflects the GatewayAPI WAF data-plane extension (design-25): its audit events flow through
// Felix's WAF event log, so it shares the WAFEventLogsFileEnabled toggle with the ApplicationLayer WAF.
func (r *ReconcileApplicationLayer) patchFelixConfiguration(ctx context.Context, al *operatorv1.ApplicationLayer, gatewayWAFEnabled bool) error {
// Fetch the Istio CR and Installation variant so DesiredPolicySyncPathPrefix
// can see whether the istio side still needs the field. Both reads tolerate
// NotFound — the istio side has no claim if either is absent.
istioCR, err := utils.GetIstio(ctx, r.client)
if err != nil {
return err
}
istioNeeds := utils.IstioRequiresPolicySync(istioCR, r.variant)

_, err = utils.PatchFelixConfiguration(ctx, r.client, func(fc *v3.FelixConfiguration) (bool, error) {
wafEventLogsFileEnabled := wafEventLogsFileRequired(al, gatewayWAFEnabled)

var tproxyMode string
if ok, v := r.getTProxyMode(al); ok {
tproxyMode = v
} else {
if fc.Spec.TPROXYMode == "" {
// Workaround: we'd like to always force the value to be the correct one, matching the operator's
// configuration. However, during an upgrade from a version that predates the TPROXYMode option,
// Felix hits a bug and gets confused by the new config parameter, which in turn triggers a restart.
// Work around that by relying on Disabled being the default value for the field instead.
//
// The felix bug was fixed in v3.16, v3.15.1 and v3.14.4; it should be safe to set new config fields
// once we know we're only upgrading from those versions and above.
//
// WAFEventLogsFileEnabled is an independent field: still enable it when a WAF producer
// (ApplicationLayer or the gateway data plane) requires it, without touching TPROXYMode.
if wafEventLogsFileEnabled && (fc.Spec.WAFEventLogsFileEnabled == nil || !*fc.Spec.WAFEventLogsFileEnabled) {
fc.Spec.WAFEventLogsFileEnabled = &wafEventLogsFileEnabled
log.Info("Patching FelixConfiguration: ", "wafEventLogsFileEnabled", wafEventLogsFileEnabled)
return true, nil
}
return false, nil
}

// If the mode is already set, fall through to the normal logic, it's safe to force-set the field now.
// This also avoids churning the config if a previous version of the operator set it to Disabled already,
// we avoid setting it back to nil.
tproxyMode = "Disabled"
// applicationLayerFieldManager owns the FelixConfiguration fields the application layer sets.
const applicationLayerFieldManager = "application-layer"

// declareApplicationLayerFields declares the fields the application layer drives: the WAF event log
// toggle it shares with the gateway data plane, and Felix's tproxy mode.
func (r *ReconcileApplicationLayer) declareApplicationLayerFields(al *operatorv1.ApplicationLayer, gatewayWAFEnabled bool) sharedconfig.DeclareFelixConfiguration {
return func(_ *v3.FelixConfiguration) (*sharedconfig.FelixConfigurationDeclaration, error) {
d := &sharedconfig.FelixConfigurationDeclaration{
Manager: applicationLayerFieldManager,
Owned: &v3.FelixConfiguration{},
Policies: map[string]sharedconfig.ConflictPolicy{
"spec.wafEventLogsFileEnabled": sharedconfig.ConflictOverride,
"spec.tproxyMode": sharedconfig.ConflictOverride,
},
}

policySyncPrefix := r.getPolicySyncPathPrefix(&fc.Spec, al, istioNeeds)
policySyncPrefixSetDesired := fc.Spec.PolicySyncPathPrefix == policySyncPrefix
tproxyModeSetDesired := fc.Spec.TPROXYMode != "" && fc.Spec.TPROXYMode == string(tproxyMode)
wafEventLogsFileEnabledDesired := fc.Spec.WAFEventLogsFileEnabled != nil && *fc.Spec.WAFEventLogsFileEnabled == wafEventLogsFileEnabled

// If tproxy mode is already set to desired state return false to indicate patch not needed.
if policySyncPrefixSetDesired && tproxyModeSetDesired && wafEventLogsFileEnabledDesired {
return false, nil
// Both fields are declared without a value when nothing asks for them, which clears them
// rather than pinning Felix to the disabled setting.
if enabled := wafEventLogsFileRequired(al, gatewayWAFEnabled); enabled {
d.Owned.Spec.WAFEventLogsFileEnabled = &enabled
}
if ok, mode := r.getTProxyMode(al); ok {
d.Owned.Spec.TPROXYMode = mode
}
return d, nil
}
}

fc.Spec.TPROXYMode = string(tproxyMode)
fc.Spec.PolicySyncPathPrefix = policySyncPrefix
fc.Spec.WAFEventLogsFileEnabled = &wafEventLogsFileEnabled

log.Info(
"Patching FelixConfiguration: ",
"policySyncPathPrefix", fc.Spec.PolicySyncPathPrefix,
"tproxyMode", string(tproxyMode),
"wafEventLogsFileEnabled", wafEventLogsFileEnabled,
)
return true, nil
})
// patchFelixConfiguration writes the fields the application layer drives.
func (r *ReconcileApplicationLayer) patchFelixConfiguration(ctx context.Context, al *operatorv1.ApplicationLayer, gatewayWAFEnabled bool) error {
writer := sharedconfig.NewWriter(r.client, r.useV3CRDs)

if _, err := writer.ApplyFelixConfiguration(ctx, r.declareApplicationLayerFields(al, gatewayWAFEnabled)); err != nil {
return err
}
// TODO(CORE-13394): drop the client here by having each feature apply the path under its own
// field manager, so no declaration has to read the other features' resources.
_, err := writer.ApplyFelixConfiguration(ctx, sharedconfig.DeclarePolicySyncPathPrefix(ctx, r.client))
return err
}

Expand All @@ -588,8 +554,8 @@ func wafEventLogsFileRequired(al *operatorv1.ApplicationLayer, gatewayWAFEnabled
}

// isGatewayWAFEnabled reports whether the GatewayAPI WAF data-plane extension is enabled. A missing
// GatewayAPI CR is treated as disabled (no error); any other read error is returned so the caller can
// requeue rather than spuriously treating WAF as disabled and flapping FelixConfiguration.
// GatewayAPI CR reads as disabled; any other read error goes back to the caller, which requeues
// rather than flapping FelixConfiguration.
func (r *ReconcileApplicationLayer) isGatewayWAFEnabled(ctx context.Context) (bool, error) {
gw, msg, err := gatewayapi.GetGatewayAPI(ctx, r.client)
if err != nil {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -150,17 +150,15 @@ var _ = Describe("Application layer controller tests", func() {
_, err = r.Reconcile(ctx, reconcile.Request{})
Expect(err).ShouldNot(HaveOccurred())

By("ensuring that felix configuration PolicySyncPathPrefix is left as is, even after ALP deletion")
By("ensuring that felix configuration PolicySyncPathPrefix is cleared after ALP deletion")
f2 := v3.FelixConfiguration{
ObjectMeta: metav1.ObjectMeta{
Name: "default",
},
}
Expect(test.GetResource(c, &f2)).To(BeNil())
// The operator-managed default is shared with egressgateway and
// Gateway API, which never clear it; the AL controller must not
// clear a value it may not own, so it is preserved here.
Expect(f2.Spec.PolicySyncPathPrefix).To(Equal("/var/run/nodeagent"))
// One field manager owns the path for every consumer, so the last one going away clears it.
Expect(f2.Spec.PolicySyncPathPrefix).To(BeEmpty())
})

It("should leave PolicySyncPathPrefix set on AL deletion when Istio CR still needs it", func() {
Expand Down Expand Up @@ -246,7 +244,7 @@ var _ = Describe("Application layer controller tests", func() {
_, err = r.Reconcile(ctx, reconcile.Request{})
Expect(err).ShouldNot(HaveOccurred())

By("ensuring that felix configuration PolicySyncPathPrefix is left as is, even after ALP deletion")
By("ensuring that a user's own PolicySyncPathPrefix survives ALP deletion")
f2 := v3.FelixConfiguration{
ObjectMeta: metav1.ObjectMeta{
Name: "default",
Expand All @@ -257,9 +255,8 @@ var _ = Describe("Application layer controller tests", func() {
})

It("should leave TPROXYMode unset if log collection is disabled", func() {
// This test verifies a workaround for upgrade from versions that don't support TPROXY to versions
// that do. Setting an unknown felix config field causes older versions of felix to cyclicly restart,
// which causes a disruptive upgrade.
// With no ApplicationLayer resource, the field is declared without a value, so Felix
// falls back to its own default rather than reading one the operator picked.
By("reconciling before without an app layer resource")
mockStatus.On("OnCRNotFound").Return()
_, err := r.Reconcile(ctx, reconcile.Request{})
Expand All @@ -276,9 +273,8 @@ var _ = Describe("Application layer controller tests", func() {
})

It("should enable WAFEventLogsFileEnabled when the GatewayAPI WAF extension is enabled (no ApplicationLayer CR)", func() {
// The gateway data-plane WAF (design-25) emits audit events that flow through Felix's WAF event
// log, so it requires the same FelixConfiguration toggle as the legacy ApplicationLayer WAF — even
// when no ApplicationLayer CR is present.
// The gateway data-plane WAF emits audit events through Felix's WAF event log, so it needs
// the same toggle as the legacy ApplicationLayer WAF, with no ApplicationLayer CR present.
mockStatus.On("OnCRNotFound").Return()

By("creating a GatewayAPI CR with the WAF extension enabled")
Expand Down Expand Up @@ -369,14 +365,14 @@ var _ = Describe("Application layer controller tests", func() {
_, err = r.Reconcile(ctx, reconcile.Request{})
Expect(err).ShouldNot(HaveOccurred())

By("ensuring that felix configuration updated to disabled")
By("ensuring that felix configuration cleared the mode")
fc = v3.FelixConfiguration{
ObjectMeta: metav1.ObjectMeta{
Name: "default",
},
}
Expect(test.GetResource(c, &fc)).To(BeNil())
Expect(fc.Spec.TPROXYMode).To(Equal("Disabled"))
Expect(fc.Spec.TPROXYMode).To(Equal(""))
})

It("should render proper SidecarWebhook status", func() {
Expand Down
46 changes: 19 additions & 27 deletions pkg/controller/egressgateway/egressgateway_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ import (

"github.com/tigera/operator/pkg/components"
"github.com/tigera/operator/pkg/controller/options"
"github.com/tigera/operator/pkg/controller/sharedconfig"
"github.com/tigera/operator/pkg/controller/status"
"github.com/tigera/operator/pkg/controller/utils"
"github.com/tigera/operator/pkg/controller/utils/imageset"
Expand Down Expand Up @@ -81,10 +82,8 @@ func newReconciler(mgr manager.Manager, opts options.ControllerOptions, licenseA
r := &ReconcileEgressGateway{
client: mgr.GetClient(),
scheme: mgr.GetScheme(),
provider: opts.DetectedProvider,
status: status.New(mgr.GetClient(), "egressgateway", opts.KubernetesVersion),
clusterDomain: opts.ClusterDomain,
variant: opts.Variant,
opts: opts,
licenseAPIReady: licenseAPIReady,
}
r.status.Run(opts.ShutdownContext)
Expand Down Expand Up @@ -130,10 +129,8 @@ type ReconcileEgressGateway struct {
// that reads objects from the cache and writes to the apiserver.
client client.Client
scheme *runtime.Scheme
provider operatorv1.Provider
status status.StatusManager
clusterDomain string
variant operatorv1.ProductVariant
opts options.ControllerOptions
licenseAPIReady *utils.ReadyFlag
}

Expand All @@ -151,11 +148,23 @@ func (r *ReconcileEgressGateway) Reconcile(ctx context.Context, request reconcil
return reconcile.Result{}, err
}

// Ahead of every early return below, because the last egress gateway going away is what
// clears the policy sync path.
fc, err := sharedconfig.NewWriter(r.client, r.opts.UseV3CRDs).ApplyFelixConfiguration(ctx, sharedconfig.DeclarePolicySyncPathPrefix(ctx, r.client))
if err != nil {
reqLogger.Error(err, "Error patching felix configuration")
r.status.SetDegraded(operatorv1.ResourcePatchError, "Error patching felix configuration", err, reqLogger)
for _, egw := range egws {
setDegraded(r.client, ctx, &egw, reconcileErr, fmt.Sprintf("Error patching felix configuration err = %s", err.Error()))
}
return reconcile.Result{}, err
}

// If there are no Egress Gateway resources, return.
ch := utils.NewComponentHandler(log, r.client, r.scheme, nil)
if len(egws) == 0 {
var objects []client.Object
if r.provider.IsOpenShift() {
if r.opts.DetectedProvider.IsOpenShift() {
objects = append(objects, egressgateway.SecurityContextConstraints())
}
err := ch.CreateOrUpdateOrDelete(ctx, render.NewDeletionPassthrough(objects...), r.status)
Expand Down Expand Up @@ -194,7 +203,7 @@ func (r *ReconcileEgressGateway) Reconcile(ctx context.Context, request reconcil
// In the case of OpenShift, we are using a single SCC.
// Whenever a EGW resource is deleted, remove the corresponding user from the SCC
// and update the resource.
if r.provider.IsOpenShift() {
if r.opts.DetectedProvider.IsOpenShift() {
scc, err := getOpenShiftSCC(ctx, r.client)
if err != nil {
reqLogger.Error(err, "Error querying SecurityContextConstraints")
Expand Down Expand Up @@ -287,27 +296,10 @@ func (r *ReconcileEgressGateway) Reconcile(ctx context.Context, request reconcil
return reconcile.Result{}, err
}

// patch and get the felix configuration
fc, err := utils.PatchFelixConfiguration(ctx, r.client, func(fc *v3.FelixConfiguration) (bool, error) {
if fc.Spec.PolicySyncPathPrefix != "" {
return false, nil // don't proceed with the patch
}
fc.Spec.PolicySyncPathPrefix = "/var/run/nodeagent"
return true, nil // proceed with this patch
})
if err != nil {
reqLogger.Error(err, "Error patching felix configuration")
r.status.SetDegraded(operatorv1.ResourcePatchError, "Error patching felix configuration", err, reqLogger)
for _, egw := range egwsToReconcile {
setDegraded(r.client, ctx, &egw, reconcileErr, fmt.Sprintf("Error patching felix configuration err = %s", err.Error()))
}
return reconcile.Result{}, err
}

// Reconcile all the EGWs
var errMsgs []string
for _, egw := range egwsToReconcile {
err = r.reconcileEgressGateway(ctx, &egw, reqLogger, r.variant, fc, pullSecrets, installationSpec, namespaceAndNames)
err = r.reconcileEgressGateway(ctx, &egw, reqLogger, r.opts.Variant, fc, pullSecrets, installationSpec, namespaceAndNames)
if err != nil {
reqLogger.Error(err, "Error reconciling egress gateway")
errMsgs = append(errMsgs, err.Error())
Expand Down Expand Up @@ -382,7 +374,7 @@ func (r *ReconcileEgressGateway) reconcileEgressGateway(ctx context.Context, egw
VXLANPort: egwVXLANPort,
VXLANVNI: egwVXLANVNI,
IptablesBackend: ipTablesBackend,
OpenShift: r.provider.IsOpenShift(),
OpenShift: r.opts.DetectedProvider.IsOpenShift(),
NamespaceAndNames: namespaceAndNames,
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -418,7 +418,7 @@ var _ = Describe("Egress Gateway controller tests", func() {
mockStatus.On("ReadyToMonitor")
Expect(c.Create(ctx, installation)).NotTo(HaveOccurred())

r.provider = operatorv1.ProviderOpenShift
r.opts.DetectedProvider = operatorv1.ProviderOpenShift
logSeverity := operatorv1.LogSeverityInfo
egw_red := &operatorv1.EgressGateway{
ObjectMeta: metav1.ObjectMeta{Name: "calico-red", Namespace: "calico-egress"},
Expand Down
Loading
Loading