Skip to content

Commit

Permalink
Merge tag '1.22.5' into tetratefips-release-1.22
Browse files Browse the repository at this point in the history 
Istio release 1.22.5
  • Loading branch information
github-actions committed Sep 21, 2024
2 parents f8729a3 + f57a44a commit 8914171
Show file tree
Hide file tree
Showing 32 changed files with 804 additions and 557 deletions.
2 changes: 1 addition & 1 deletion Makefile.core.mk
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ endif
export VERSION

# Base version of Istio image to use
BASE_VERSION ?= 1.22-2024-08-08T19-01-18
BASE_VERSION ?= 1.22-2024-09-04T19-02-08
ISTIO_BASE_REGISTRY ?= gcr.io/istio-release

export GO111MODULE ?= on
Expand Down
2 changes: 1 addition & 1 deletion istio.deps
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
"name": "PROXY_REPO_SHA",
"repoName": "proxy",
"file": "",
"lastStableSHA": "9603bcb6fd358cb0affbb662630751d77ab66efb"
"lastStableSHA": "f190ab5d663a808ed8e4fea58eba4447cd51f2a7"
},
{
"_comment": "",
Expand Down
30 changes: 21 additions & 9 deletions pilot/pkg/grpc/grpc.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,28 +102,40 @@ func containsExpectedMessage(msg string) bool {
return false
}

// IsExpectedGRPCError checks a gRPC error code and determines whether it is an expected error when
// things are operating normally. This is basically capturing when the client disconnects.
func IsExpectedGRPCError(err error) bool {
type ErrorType string

const (
// This indicates all the errors except the expected errors or graceful termination.
UnexpectedError ErrorType = "unexpectedError"
// This indicates an expected error when things are operating normally.
ExpectedError ErrorType = "expectedError"
// This indicates an error which happen when the connection is gracefully terminated.
// For example, the peer calls `SendClose()`.
GracefulTermination ErrorType = "gracefulTermination"
)

// GRPCErrorType checks a gRPC error code and determines its ErrorType.
// This is basically capturing when the peer disconnects.
func GRPCErrorType(err error) ErrorType {
if err == io.EOF {
return true
return GracefulTermination
}

if s, ok := status.FromError(err); ok {
if s.Code() == codes.Canceled || s.Code() == codes.DeadlineExceeded {
return true
return ExpectedError
}
if s.Code() == codes.Unavailable && containsExpectedMessage(s.Message()) {
return true
return ExpectedError
}
}
// If this is not a gRPCStatus we should just error message.
if strings.Contains(err.Error(), "stream terminated by RST_STREAM with error code: NO_ERROR") {
return true
return ExpectedError
}
if strings.Contains(err.Error(), "received prior goaway: code: NO_ERROR") {
return true
return ExpectedError
}

return false
return ExpectedError
}
26 changes: 23 additions & 3 deletions pilot/pkg/grpc/grpc_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,32 @@ package grpc

import (
"errors"
"io"
"testing"
)

func TestIsExpectedGRPCError(t *testing.T) {
err := errors.New("code = Internal desc = stream terminated by RST_STREAM with error code: NO_ERROR")
if got := IsExpectedGRPCError(err); !got {
t.Fatalf("expected true, got %v", got)
tests := []struct {
name string
err error
want ErrorType
}{
{
name: "RST_STREAM",
err: errors.New("code = Internal desc = stream terminated by RST_STREAM with error code: NO_ERROR"),
want: ExpectedError,
},
{
name: "EOF",
err: io.EOF,
want: GracefulTermination,
},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
if got := GRPCErrorType(tc.err); got != tc.want {
t.Fatalf("expected got %v, but want: %v", got, tc.want)
}
})
}
}
64 changes: 7 additions & 57 deletions pilot/pkg/model/service.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -587,7 +587,7 @@ func (ep *IstioEndpoint) CmpOpts() []cmp.Option {

// Key returns a function suitable for usage to distinguish this IstioEndpoint from another
func (ep *IstioEndpoint) Key() string {
return ep.Address + "/" + ep.WorkloadName
return ep.Address + "/" + ep.WorkloadName + "/" + ep.ServicePortName
}

// EndpointMetadata represents metadata set on Envoy LbEndpoint used for telemetry purposes.
Expand Down Expand Up @@ -1270,33 +1270,14 @@ func (s *Service) GetAddressForProxy(node *Proxy) string {
// GetExtraAddressesForProxy returns a k8s service's extra addresses to the cluster where the node resides.
// Especially for dual stack k8s service to get other IP family addresses.
func (s *Service) GetExtraAddressesForProxy(node *Proxy) []string {
addresses := s.getAllAddressesForProxy(node)
if len(addresses) > 1 {
return addresses[1:]
}
return nil
}

// GetAllAddressesForProxy returns a k8s service's extra addresses to the cluster where the node resides.
// Especially for dual stack k8s service to get other IP family addresses.
func (s *Service) GetAllAddressesForProxy(node *Proxy) []string {
return s.getAllAddressesForProxy(node)
}

func (s *Service) getAllAddressesForProxy(node *Proxy) []string {
if node.Metadata != nil && node.Metadata.ClusterID != "" {
addresses := s.ClusterVIPs.GetAddressesFor(node.Metadata.ClusterID)
if (features.EnableDualStack || features.EnableAmbient) && len(addresses) > 0 {
return addresses
}
addresses = filterAddresses(addresses, node.SupportsIPv4(), node.SupportsIPv6())
if len(addresses) > 0 {
return addresses
if features.EnableDualStack && node.Metadata != nil {
if node.Metadata.ClusterID != "" {
addresses := s.ClusterVIPs.GetAddressesFor(node.Metadata.ClusterID)
if len(addresses) > 1 {
return addresses[1:]
}
}
}
if a := s.GetAddressForProxy(node); a != "" {
return []string{a}
}
return nil
}

Expand All @@ -1311,37 +1292,6 @@ func (s *Service) getAllAddresses() []string {
return addresses
}

func filterAddresses(addresses []string, supportsV4, supportsV6 bool) []string {
var ipv4Addresses []string
var ipv6Addresses []string
for _, addr := range addresses {
// check if an address is a CIDR range
if strings.Contains(addr, "/") {
if prefix, err := netip.ParsePrefix(addr); err != nil {
log.Warnf("failed to parse prefix address '%s': %s", addr, err)
continue
} else if supportsV4 && prefix.Addr().Is4() {
ipv4Addresses = append(ipv4Addresses, addr)
} else if supportsV6 && prefix.Addr().Is6() {
ipv6Addresses = append(ipv6Addresses, addr)
}
} else {
if ipAddr, err := netip.ParseAddr(addr); err != nil {
log.Warnf("failed to parse address '%s': %s", addr, err)
continue
} else if supportsV4 && ipAddr.Is4() {
ipv4Addresses = append(ipv4Addresses, addr)
} else if supportsV6 && ipAddr.Is6() {
ipv6Addresses = append(ipv6Addresses, addr)
}
}
}
if len(ipv4Addresses) > 0 {
return ipv4Addresses
}
return ipv6Addresses
}

// GetTLSModeFromEndpointLabels returns the value of the label
// security.istio.io/tlsMode if set. Do not return Enums or constants
// from this function as users could provide values other than istio/disabled
Expand Down
144 changes: 0 additions & 144 deletions pilot/pkg/model/service_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,11 @@ import (
"github.com/google/go-cmp/cmp/cmpopts"
fuzz "github.com/google/gofuzz"

"istio.io/istio/pilot/pkg/features"
"istio.io/istio/pkg/cluster"
"istio.io/istio/pkg/config/constants"
"istio.io/istio/pkg/config/host"
"istio.io/istio/pkg/config/labels"
"istio.io/istio/pkg/config/visibility"
"istio.io/istio/pkg/test"
"istio.io/istio/pkg/test/util/assert"
)

Expand Down Expand Up @@ -588,145 +586,3 @@ func TestParseSubsetKeyHostname(t *testing.T) {
})
}
}

func TestGetAllAddresses(t *testing.T) {
tests := []struct {
name string
service *Service
ipMode IPMode
dualStackEnabled bool
ambientEnabled bool
autoAllocationEnabled bool
expectedAddresses []string
expectedExtraAddresses []string
}{
{
name: "IPv4 mode, IPv4 and IPv6 CIDR addresses, expected to return only IPv4 addresses",
service: &Service{
DefaultAddress: "10.0.0.0/28",
ClusterVIPs: AddressMap{
Addresses: map[cluster.ID][]string{
"id": {"10.0.0.0/28", "10.0.0.16/28", "::ffff:10.0.0.32/96", "::ffff:10.0.0.48/96"},
},
},
},
ipMode: IPv4,
expectedAddresses: []string{"10.0.0.0/28", "10.0.0.16/28"},
expectedExtraAddresses: []string{"10.0.0.16/28"},
},
{
name: "IPv6 mode, IPv4 and IPv6 CIDR addresses, expected to return only IPv6 addresses",
service: &Service{
DefaultAddress: "10.0.0.0/28",
ClusterVIPs: AddressMap{
Addresses: map[cluster.ID][]string{
"id": {"10.0.0.0/28", "10.0.0.16/28", "::ffff:10.0.0.32/96", "::ffff:10.0.0.48/96"},
},
},
},
ipMode: IPv6,
expectedAddresses: []string{"::ffff:10.0.0.32/96", "::ffff:10.0.0.48/96"},
expectedExtraAddresses: []string{"::ffff:10.0.0.48/96"},
},
{
name: "dual mode, ISTIO_DUAL_STACK disabled, IPv4 and IPv6 addresses, expected to return only IPv4 addresses",
service: &Service{
DefaultAddress: "10.0.0.0",
ClusterVIPs: AddressMap{
Addresses: map[cluster.ID][]string{
"id": {"10.0.0.0", "10.0.0.16", "::ffff:10.0.0.32", "::ffff:10.0.0.48"},
},
},
},
ipMode: Dual,
expectedAddresses: []string{"10.0.0.0", "10.0.0.16"},
expectedExtraAddresses: []string{"10.0.0.16"},
},
{
name: "dual mode, ISTIO_DUAL_STACK enabled, IPv4 and IPv6 addresses, expected to return only IPv4 addresses",
service: &Service{
DefaultAddress: "10.0.0.0",
ClusterVIPs: AddressMap{
Addresses: map[cluster.ID][]string{
"id": {"10.0.0.0", "10.0.0.16", "::ffff:10.0.0.32", "::ffff:10.0.0.48"},
},
},
},
ipMode: Dual,
dualStackEnabled: true,
expectedAddresses: []string{"10.0.0.0", "10.0.0.16", "::ffff:10.0.0.32", "::ffff:10.0.0.48"},
expectedExtraAddresses: []string{"10.0.0.16", "::ffff:10.0.0.32", "::ffff:10.0.0.48"},
},
{
name: "IPv4 mode, ISTIO_DUAL_STACK disabled, ambient enabled, IPv4 and IPv6 addresses, expected to return all addresses",
service: &Service{
DefaultAddress: "10.0.0.0/28",
ClusterVIPs: AddressMap{
Addresses: map[cluster.ID][]string{
"id": {"10.0.0.0/28", "10.0.0.16/28", "::ffff:10.0.0.32", "::ffff:10.0.0.48"},
},
},
},
ipMode: IPv4,
ambientEnabled: true,
expectedAddresses: []string{"10.0.0.0/28", "10.0.0.16/28", "::ffff:10.0.0.32", "::ffff:10.0.0.48"},
expectedExtraAddresses: []string{"10.0.0.16/28", "::ffff:10.0.0.32", "::ffff:10.0.0.48"},
},
{
name: "IPv6 mode, ISTIO_DUAL_STACK disabled, ambient enabled, IPv4 and IPv6 addresses, expected to return all addresses",
service: &Service{
DefaultAddress: "10.0.0.0/28",
ClusterVIPs: AddressMap{
Addresses: map[cluster.ID][]string{
"id": {"10.0.0.0/28", "10.0.0.16/28", "::ffff:10.0.0.32", "::ffff:10.0.0.48"},
},
},
},
ipMode: IPv6,
ambientEnabled: true,
expectedAddresses: []string{"10.0.0.0/28", "10.0.0.16/28", "::ffff:10.0.0.32", "::ffff:10.0.0.48"},
expectedExtraAddresses: []string{"10.0.0.16/28", "::ffff:10.0.0.32", "::ffff:10.0.0.48"},
},
{
name: "IPv4 mode, auto-allocation enabled, expected auto-allocated address",
service: &Service{
DefaultAddress: "0.0.0.0",
AutoAllocatedIPv4Address: "240.240.0.1",
},
ipMode: IPv4,
autoAllocationEnabled: true,
expectedAddresses: []string{"240.240.0.1"},
expectedExtraAddresses: []string{},
},
{
name: "IPv6 mode, auto-allocation enabled, expected auto-allocated address",
service: &Service{
DefaultAddress: "0.0.0.0",
AutoAllocatedIPv6Address: "2001:2::f0f0:e351",
},
ipMode: IPv6,
autoAllocationEnabled: true,
expectedAddresses: []string{"2001:2::f0f0:e351"},
expectedExtraAddresses: []string{},
},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
if tc.dualStackEnabled {
test.SetForTest(t, &features.EnableDualStack, true)
}
if tc.ambientEnabled {
test.SetForTest(t, &features.EnableAmbient, true)
}
proxy := &Proxy{Metadata: &NodeMetadata{ClusterID: "id"}, ipMode: tc.ipMode}
if tc.autoAllocationEnabled {
proxy.Metadata.DNSCapture = true
proxy.Metadata.DNSAutoAllocate = true
}
addresses := tc.service.GetAllAddressesForProxy(proxy)
assert.Equal(t, addresses, tc.expectedAddresses)
extraAddresses := tc.service.GetExtraAddressesForProxy(proxy)
assert.Equal(t, extraAddresses, tc.expectedExtraAddresses)
})
}
}
11 changes: 6 additions & 5 deletions pilot/pkg/networking/core/listener.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -264,10 +264,10 @@ func (l listenerBinding) Primary() string {

// Extra returns any additional bindings. This is always empty if dual stack is disabled
func (l listenerBinding) Extra() []string {
if len(l.binds) > 1 {
return l.binds[1:]
if !features.EnableDualStack || len(l.binds) == 1 {
return nil
}
return nil
return l.binds[1:]
}

type outboundListenerEntry struct {
Expand Down Expand Up @@ -820,8 +820,9 @@ func (lb *ListenerBuilder) buildSidecarOutboundListener(listenerOpts outboundLis
} else {
// Address is a CIDR. Fall back to 0.0.0.0 and
// filter chain match
// TODO: this probably needs to handle dual stack better
listenerOpts.bind.binds = actualWildcards
listenerOpts.cidr = append([]string{svcListenAddress}, svcExtraListenAddresses...)
listenerOpts.cidr = svcListenAddress
}
}
}
Expand Down Expand Up @@ -1060,7 +1061,7 @@ type outboundListenerOpts struct {
proxy *model.Proxy

bind listenerBinding
cidr []string
cidr string

port *model.Port
service *model.Service
Expand Down
5 changes: 4 additions & 1 deletion pilot/pkg/networking/core/listener_builder.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -421,7 +421,10 @@ func (lb *ListenerBuilder) buildHTTPConnectionManager(httpOpts *httpListenerOpts
connectionManager.HttpFilters = filters
connectionManager.RequestIdExtension = requestidextension.BuildUUIDRequestIDExtension(reqIDExtensionCtx)

if features.EnableHCMInternalNetworks && lb.push.Networks != nil {
// If UseRemoteAddress is set, we must set the internal address config in preparation for envoy
// internal addresses defaulting to empty set. Currently, the internal addresses defaulted to
// all private IPs but this will change in the future.
if (features.EnableHCMInternalNetworks || httpOpts.useRemoteAddress) && lb.push.Networks != nil {
for _, internalnetwork := range lb.push.Networks.Networks {
iac := &hcm.HttpConnectionManager_InternalAddressConfig{}
for _, ne := range internalnetwork.Endpoints {
Expand Down
Loading

0 comments on commit 8914171

Please sign in to comment.