Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

v1.13 Backports 2024-03-19 #31496

Merged
merged 9 commits into from Mar 21, 2024
1 change: 1 addition & 0 deletions .github/actions/lvh-kind/action.yaml
Expand Up @@ -28,6 +28,7 @@ runs:
mem: 12G
install-dependencies: 'true'
port-forward: '6443:6443'
ssh-startup-wait-retries: 600
cmd: |
git config --global --add safe.directory /host

Expand Down
1 change: 1 addition & 0 deletions .github/workflows/integration-test.yaml
Expand Up @@ -66,6 +66,7 @@ jobs:
integration-test:
name: Integration Test
strategy:
fail-fast: false
matrix:
arch: [ubuntu-22.04, ubuntu-22.04-arm64]
runs-on: ${{ matrix.arch }}
Expand Down
@@ -1,9 +1,10 @@
Prerequisites
#############

* Cilium must be configured with ``kubeProxyReplacement`` as partial
or strict. Please refer to :ref:`kube-proxy replacement <kubeproxy-free>`
for more details.
* Cilium must be configured with NodePort enabled, using
``nodePort.enabled=true`` or by enabling the kube-proxy replacement with
``kubeProxyReplacement`` as partial or strict. For more information,
see :ref:`kube-proxy replacement <kubeproxy-free>`.
* Cilium must be configured with the L7 proxy enabled using the ``--enable-l7-proxy`` flag (enabled by default).
* The below CRDs from Gateway API v0.5.1 ``must`` be pre-installed.
Please refer to this `docs <https://gateway-api.sigs.k8s.io/guides/?h=crds#getting-started-with-gateway-api>`_
Expand Down
6 changes: 6 additions & 0 deletions Documentation/security/network/encryption-ipsec.rst
Expand Up @@ -162,6 +162,12 @@ commands:
Key Rotation
============

.. attention::

Key rotations should not be performed during upgrades and downgrades. That
is, all nodes in the cluster (or clustermesh) should be on the same Cilium
version before rotating keys.

To replace cilium-ipsec-keys secret with a new key:

.. code-block:: shell-session
Expand Down
2 changes: 1 addition & 1 deletion operator/pkg/gateway-api/gateway_reconcile.go
Expand Up @@ -239,7 +239,7 @@ func (r *gatewayReconciler) setAddressStatus(ctx context.Context, gw *gatewayv1b
svcList := &corev1.ServiceList{}
if err := r.Client.List(ctx, svcList, client.MatchingLabels{
owningGatewayLabel: gw.GetName(),
}); err != nil {
}, client.InNamespace(gw.GetNamespace())); err != nil {
return err
}

Expand Down
69 changes: 39 additions & 30 deletions operator/pkg/gateway-api/gateway_reconcile_test.go
Expand Up @@ -37,7 +37,30 @@ var gwFixture = []client.Object{
Namespace: "default",
},
},

&corev1.Service{
ObjectMeta: metav1.ObjectMeta{
Name: "cilium-gateway-valid-gateway",
Namespace: "another-namespace",
Annotations: map[string]string{
"pre-existing-annotation": "true",
},
},
Status: corev1.ServiceStatus{
LoadBalancer: corev1.LoadBalancerStatus{
Ingress: []corev1.LoadBalancerIngress{
{
IP: "10.10.10.11",
Ports: []corev1.PortStatus{
{
Port: 80,
Protocol: "TCP",
},
},
},
},
},
},
},
&corev1.Service{
ObjectMeta: metav1.ObjectMeta{
Name: "cilium-gateway-valid-gateway",
Expand All @@ -46,6 +69,21 @@ var gwFixture = []client.Object{
"pre-existing-annotation": "true",
},
},
Status: corev1.ServiceStatus{
LoadBalancer: corev1.LoadBalancerStatus{
Ingress: []corev1.LoadBalancerIngress{
{
IP: "10.10.10.10",
Ports: []corev1.PortStatus{
{
Port: 80,
Protocol: "TCP",
},
},
},
},
},
},
},

// Service in another namespace
Expand Down Expand Up @@ -199,35 +237,6 @@ func Test_gatewayReconciler_Reconcile(t *testing.T) {
result, err := r.Reconcile(context.Background(), ctrl.Request{NamespacedName: key})

// First reconcile should wait for LB status
require.Error(t, err)
require.Equal(t, "load balancer status is not ready", err.Error())
require.Equal(t, ctrl.Result{}, result)

// Simulate LB service update
lb := &corev1.Service{}
err = c.Get(context.Background(), client.ObjectKey{Namespace: "default", Name: "cilium-gateway-valid-gateway"}, lb)
require.NoError(t, err)
require.Equal(t, corev1.ServiceTypeLoadBalancer, lb.Spec.Type)
require.Equal(t, "valid-gateway", lb.Labels["io.cilium.gateway/owning-gateway"])
require.Equal(t, "true", lb.Annotations["pre-existing-annotation"])

// Update LB status
lb.Status.LoadBalancer.Ingress = []corev1.LoadBalancerIngress{
{
IP: "10.10.10.10",
Ports: []corev1.PortStatus{
{
Port: 80,
Protocol: "TCP",
},
},
},
}
err = c.Status().Update(context.Background(), lb)
require.NoError(t, err)

// Perform second reconciliation
result, err = r.Reconcile(context.Background(), ctrl.Request{NamespacedName: key})
require.NoError(t, err)
require.Equal(t, ctrl.Result{}, result)

Expand Down
4 changes: 4 additions & 0 deletions pkg/alibabacloud/eni/types/types.go
Expand Up @@ -126,6 +126,10 @@ type ENI struct {
Tags map[string]string `json:"tags,omitempty"`
}

func (e *ENI) DeepCopyInterface() types.Interface {
return e.DeepCopy()
}

// InterfaceID returns the identifier of the interface
func (e *ENI) InterfaceID() string {
return e.NetworkInterfaceID
Expand Down
12 changes: 12 additions & 0 deletions pkg/aws/ec2/ec2.go
Expand Up @@ -27,6 +27,18 @@ import (
"github.com/cilium/cilium/pkg/spanstat"
)

const (
SubnetFullErrMsgStr = "There aren't sufficient free Ipv4 addresses or prefixes"

// InsufficientPrefixesInSubnetStr AWS error code for insufficient /28 prefixes in a subnet, possibly due to
// fragmentation
InsufficientPrefixesInSubnetStr = "InsufficientCidrBlocks"

// InvalidParameterValueStr sort of catch-all error code from AWS to indicate request params are invalid. Often,
// requires looking at the error message to get the actual reason. See SubnetFullErrMsgStr for example.
InvalidParameterValueStr = "InvalidParameterValue"
)

// Client represents an EC2 API client
type Client struct {
ec2Client *ec2.Client
Expand Down
7 changes: 6 additions & 1 deletion pkg/aws/ec2/mock/mock.go
Expand Up @@ -13,13 +13,15 @@ import (
"github.com/cilium/ipam/service/ipallocator"

"github.com/cilium/cilium/pkg/api/helpers"
"github.com/cilium/cilium/pkg/aws/ec2"
eniTypes "github.com/cilium/cilium/pkg/aws/eni/types"
"github.com/cilium/cilium/pkg/aws/types"
"github.com/cilium/cilium/pkg/ip"
"github.com/cilium/cilium/pkg/ipam/option"
ipamTypes "github.com/cilium/cilium/pkg/ipam/types"
"github.com/cilium/cilium/pkg/lock"

"github.com/aws/smithy-go"
"github.com/google/uuid"
log "github.com/sirupsen/logrus"
"golang.org/x/time/rate"
Expand Down Expand Up @@ -427,7 +429,10 @@ func assignPrefixToENI(e *API, eni *eniTypes.ENI, prefixes int32) error {
}

if int(prefixes)*option.ENIPDBlockSizeIPv4 > subnet.AvailableAddresses {
return fmt.Errorf("subnet %s has not enough addresses available", eni.Subnet.ID)
return &smithy.GenericAPIError{
Code: ec2.InvalidParameterValueStr,
Message: ec2.SubnetFullErrMsgStr,
}
}

for i := int32(0); i < prefixes; i++ {
Expand Down
11 changes: 6 additions & 5 deletions pkg/aws/eni/node.go
Expand Up @@ -19,6 +19,7 @@ import (
"github.com/sirupsen/logrus"

operatorOption "github.com/cilium/cilium/operator/option"
"github.com/cilium/cilium/pkg/aws/ec2"
"github.com/cilium/cilium/pkg/aws/eni/limits"
eniTypes "github.com/cilium/cilium/pkg/aws/eni/types"
"github.com/cilium/cilium/pkg/defaults"
Expand All @@ -38,9 +39,6 @@ const (

getMaximumAllocatableIPv4FailureWarningStr = "maximum allocatable ipv4 addresses will be 0 (unlimited)" +
" this could lead to ip allocation overflows if the max-allocate flag is not set"

// insufficientPrefixesInSubnetStr AWS error code for insufficient /28 prefixes in a subnet
insufficientPrefixesInSubnetStr = "InsufficientCidrBlocks"
)

// Node represents a Kubernetes node running Cilium with an associated
Expand Down Expand Up @@ -259,7 +257,9 @@ func (n *Node) PrepareIPAllocation(scopedLog *logrus.Entry) (a *ipam.AllocationA
func isSubnetAtPrefixCapacity(err error) bool {
var apiErr smithy.APIError
if errors.As(err, &apiErr) {
return apiErr.ErrorCode() == insufficientPrefixesInSubnetStr
return apiErr.ErrorCode() == ec2.InsufficientPrefixesInSubnetStr ||
(apiErr.ErrorCode() == ec2.InvalidParameterValueStr &&
strings.Contains(apiErr.ErrorMessage(), ec2.SubnetFullErrMsgStr))
}
return false
}
Expand Down Expand Up @@ -345,7 +345,8 @@ func (n *Node) errorInstanceNotRunning(err error) (notRunning bool) {
func isAttachmentIndexConflict(err error) bool {
var apiErr smithy.APIError
if errors.As(err, &apiErr) {
return apiErr.ErrorCode() == "InvalidParameterValue" && strings.Contains(apiErr.ErrorMessage(), "interface attached at device")
return apiErr.ErrorCode() == ec2.InvalidParameterValueStr &&
strings.Contains(apiErr.ErrorMessage(), "interface attached at device")
}
return false
}
Expand Down
20 changes: 19 additions & 1 deletion pkg/aws/eni/node_manager_test.go
Expand Up @@ -158,7 +158,8 @@ func (e *ENISuite) TestNodeManagerDefaultAllocation(c *check.C) {
func (e *ENISuite) TestNodeManagerPrefixDelegation(c *check.C) {
const instanceID = "i-testNodeManagerDefaultAllocation-0"

ec2api := ec2mock.NewAPI([]*ipamTypes.Subnet{testSubnet}, []*ipamTypes.VirtualNetwork{testVpc}, testSecurityGroups)
pdTestSubnet := *testSubnet
ec2api := ec2mock.NewAPI([]*ipamTypes.Subnet{&pdTestSubnet}, []*ipamTypes.VirtualNetwork{testVpc}, testSecurityGroups)
instances := NewInstancesManager(ec2api)
c.Assert(instances, check.Not(check.IsNil))
eniID1, _, err := ec2api.CreateNetworkInterface(context.TODO(), 0, "s-1", "desc", []string{"sg1", "sg2"}, true)
Expand Down Expand Up @@ -196,6 +197,23 @@ func (e *ENISuite) TestNodeManagerPrefixDelegation(c *check.C) {
totalPrefixes += len(eni.Prefixes)
}
c.Assert(totalPrefixes, check.Equals, 2)

// Test fallback to /32 IPs when /28 blocks aren't available
//
// Set available IPs to a value insufficient to allocate a /28 block, but enough for /32 IPs to resolve
// pre-allocate deficit.
pdTestSubnet.AvailableAddresses = 15
ec2api.UpdateSubnets([]*ipamTypes.Subnet{&pdTestSubnet})

// Use 25 out of 32 IPs
mngr.Update(updateCiliumNode(cn, 32, 25))
c.Assert(testutils.WaitUntil(func() bool { return reachedAddressesNeeded(mngr, "node1", 0) }, 5*time.Second), check.IsNil)

node = mngr.Get("node1")
c.Assert(node, check.Not(check.IsNil))
// Should allocate only 1 additional IP after fallback, not an entire /28 prefix
c.Assert(node.Stats().AvailableIPs, check.Equals, 33)
c.Assert(node.Stats().UsedIPs, check.Equals, 25)
}

// TestNodeManagerENIWithSGTags tests ENI allocation + association with a SG based on tags
Expand Down
4 changes: 4 additions & 0 deletions pkg/aws/eni/types/types.go
Expand Up @@ -208,6 +208,10 @@ type ENI struct {
Tags map[string]string `json:"tags,omitempty"`
}

func (e *ENI) DeepCopyInterface() types.Interface {
return e.DeepCopy()
}

// InterfaceID returns the identifier of the interface
func (e *ENI) InterfaceID() string {
return e.ID
Expand Down
4 changes: 4 additions & 0 deletions pkg/azure/types/types.go
Expand Up @@ -126,6 +126,10 @@ type AzureInterface struct {
resourceGroup string `json:"-"`
}

func (a *AzureInterface) DeepCopyInterface() types.Interface {
return a.DeepCopy()
}

// SetID sets the Azure interface ID, as well as extracting other fields from
// the ID itself.
func (a *AzureInterface) SetID(id string) {
Expand Down
21 changes: 21 additions & 0 deletions pkg/hubble/parser/threefour/parser_test.go
Expand Up @@ -619,6 +619,17 @@ func TestDecodeTrafficDirection(t *testing.T) {
assert.Equal(t, flowpb.TrafficDirection_TRAFFIC_DIRECTION_UNKNOWN, f.GetTrafficDirection())
assert.Equal(t, uint32(localEP), f.GetSource().GetID())

// TRACE_FROM_LXC unknown (encrypted)
tn = monitor.TraceNotifyV0{
Type: byte(monitorAPI.MessageTypeTrace),
Source: localEP,
ObsPoint: monitorAPI.TraceFromLxc,
Reason: monitor.TraceReasonUnknown | monitor.TraceReasonEncryptMask,
}
f = parseFlow(tn, localIP, remoteIP)
assert.Equal(t, flowpb.TrafficDirection_TRAFFIC_DIRECTION_UNKNOWN, f.GetTrafficDirection())
assert.Equal(t, uint32(localEP), f.GetSource().GetID())

// PolicyVerdictNotify Egress
pvn := monitor.PolicyVerdictNotify{
Type: byte(monitorAPI.MessageTypePolicyVerdict),
Expand Down Expand Up @@ -693,6 +704,16 @@ func TestDecodeIsReply(t *testing.T) {
assert.Nil(t, f.GetIsReply())
assert.Equal(t, false, f.GetReply())

// TRACE_FROM_LXC encrypted
tn = monitor.TraceNotifyV0{
Type: byte(monitorAPI.MessageTypeTrace),
ObsPoint: monitorAPI.TraceFromLxc,
Reason: monitor.TraceReasonUnknown | monitor.TraceReasonEncryptMask,
}
f = parseFlow(tn, localIP, remoteIP)
assert.Nil(t, f.GetIsReply())
assert.Equal(t, false, f.GetReply())

// PolicyVerdictNotify forward statically assumes is_reply=false
pvn := monitor.PolicyVerdictNotify{
Type: byte(monitorAPI.MessageTypePolicyVerdict),
Expand Down
4 changes: 4 additions & 0 deletions pkg/ipam/types/types.go
Expand Up @@ -295,6 +295,9 @@ type Interface interface {
// ForeachAddress must iterate over all addresses of the interface and
// call fn for each address
ForeachAddress(instanceID string, fn AddressIterator) error

// DeepCopyInterface returns a deep copy of the underlying interface type.
DeepCopyInterface() Interface
}

// InterfaceRevision is the configurationr revision of a network interface. It
Expand Down Expand Up @@ -469,6 +472,7 @@ func (m *InstanceMap) DeepCopy() *InstanceMap {
c := NewInstanceMap()
m.ForeachInterface("", func(instanceID, interfaceID string, rev InterfaceRevision) error {
// c is not exposed yet, we can access it without locking it
rev.Resource = rev.Resource.DeepCopyInterface()
c.updateLocked(instanceID, rev)
return nil
})
Expand Down
17 changes: 17 additions & 0 deletions pkg/ipam/types/types_test.go
Expand Up @@ -31,6 +31,23 @@ type mockInterface struct {
pools map[string][]net.IP
}

func (m *mockInterface) DeepCopyInterface() Interface {
mc := &mockInterface{
id: m.id,
pools: map[string][]net.IP{},
}
for id, pool := range m.pools {
pc := make([]net.IP, 0, len(pool))
for _, ip := range pool {
ipc := net.IP{}
copy(ipc, ip)
pc = append(pc, ipc)
}
mc.pools[id] = pc
}
return mc
}

func (m *mockInterface) InterfaceID() string {
return m.id
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/monitor/datapath_trace.go
Expand Up @@ -105,7 +105,7 @@ func connState(reason uint8) string {
}

func TraceReasonIsKnown(reason uint8) bool {
switch reason {
switch reason & ^TraceReasonEncryptMask {
case TraceReasonUnknown:
return false
default:
Expand Down