Skip to content

Commit

Permalink
[1.18] don't attempt to read v1 DestinationRules when istio is too old (
Browse files Browse the repository at this point in the history
  • Loading branch information
stevenctl authored Dec 10, 2024
1 parent 286f123 commit 522171f
Show file tree
Hide file tree
Showing 6 changed files with 8,429 additions and 23 deletions.
6 changes: 6 additions & 0 deletions changelog/v1.18.0-rc5/version-gate-destrule.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
changelog:
- type: FIX
issueLink: https://github.com/solo-io/solo-projects/issues/7400
resolvesIssue: true
description: >-
When the v1 DestinationRule CRD does not exist, don't attemp to read DestinationRule.
1 change: 1 addition & 0 deletions projects/gateway2/controller/start.go
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,7 @@ func NewControllerBuilder(ctx context.Context, cfg StartConfig) (*ControllerBuil
ctx,
cfg.InitialSettings,
cfg.Settings,
cfg.RestConfig,
wellknown.GatewayControllerName,
setup.GetWriteNamespace(&cfg.InitialSettings.Spec),
inputChannels,
Expand Down
37 changes: 29 additions & 8 deletions projects/gateway2/proxy_syncer/destrule.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
package proxy_syncer

import (
"context"
"fmt"
"slices"

"github.com/solo-io/gloo/pkg/schemes"
"github.com/solo-io/go-utils/contextutils"
"google.golang.org/protobuf/proto"
"istio.io/api/networking/v1alpha3"
networkingclient "istio.io/client-go/pkg/apis/networking/v1"
Expand All @@ -12,6 +15,7 @@ import (
"istio.io/istio/pkg/kube/kclient"
"istio.io/istio/pkg/kube/krt"
"istio.io/istio/pkg/kube/kubetypes"
"k8s.io/client-go/rest"
)

type NsWithHostname struct {
Expand Down Expand Up @@ -54,15 +58,32 @@ func (c DestinationRuleWrapper) Equals(k DestinationRuleWrapper) bool {
return proto.Equal(&c.Spec, &k.Spec)
}

func NewDestRuleIndex(istioClient kube.Client, dbg *krt.DebugHandler) DestinationRuleIndex {
destRuleClient := kclient.NewDelayedInformer[*networkingclient.DestinationRule](istioClient, gvr.DestinationRule, kubetypes.StandardInformer, kclient.Filter{})
rawDestrules := krt.WrapClient(destRuleClient, krt.WithName("DestinationRules"), krt.WithDebugging(dbg))
destrules := krt.NewCollection(rawDestrules, func(kctx krt.HandlerContext, dr *networkingclient.DestinationRule) *DestinationRuleWrapper {
return &DestinationRuleWrapper{dr}
})
func NewDestRuleIndex(
ctx context.Context,
restCfg *rest.Config,
istioClient kube.Client,
dbg *krt.DebugHandler,
) DestinationRuleIndex {
var wrappedDestRules krt.Collection[DestinationRuleWrapper]
if ok, err := schemes.CRDExists(
restCfg,
networkingclient.SchemeGroupVersion.Group,
networkingclient.SchemeGroupVersion.Version,
"DestinationRule",
); ok && err == nil {
destRuleClient := kclient.NewDelayedInformer[*networkingclient.DestinationRule](istioClient, gvr.DestinationRule, kubetypes.StandardInformer, kclient.Filter{})
rawDestrules := krt.WrapClient(destRuleClient, krt.WithName("DestinationRules"), krt.WithDebugging(dbg))
wrappedDestRules = krt.NewCollection(rawDestrules, func(kctx krt.HandlerContext, dr *networkingclient.DestinationRule) *DestinationRuleWrapper {
return &DestinationRuleWrapper{dr}
})
} else {
contextutils.LoggerFrom(ctx).Warn("DestinatonRule v1 CRD not found; running without DestinationRule support")
wrappedDestRules = krt.NewStaticCollection[DestinationRuleWrapper]([]DestinationRuleWrapper{})
}

return DestinationRuleIndex{
Destrules: destrules,
ByHostname: newDestruleIndex(destrules),
Destrules: wrappedDestRules,
ByHostname: newDestruleIndex(wrappedDestRules),
}
}

Expand Down
16 changes: 12 additions & 4 deletions projects/gateway2/proxy_syncer/proxy_syncer.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/client-go/rest"
"k8s.io/client-go/tools/cache"

rlexternal "github.com/solo-io/gloo/projects/gloo/api/external/solo/ratelimit"
Expand Down Expand Up @@ -75,6 +76,7 @@ type ProxySyncer struct {
initialSettings *glookubev1.Settings
inputs *GatewayInputChannels
mgr manager.Manager
restCfg *rest.Config
k8sGwExtensions extensions.K8sGatewayExtensions

proxyTranslator ProxyTranslator
Expand Down Expand Up @@ -117,6 +119,7 @@ func NewProxySyncer(
ctx context.Context,
initialSettings *glookubev1.Settings,
settings krt.Singleton[glookubev1.Settings],
restCfg *rest.Config,
controllerName string,
writeNamespace string,
inputs *GatewayInputChannels,
Expand All @@ -137,6 +140,7 @@ func NewProxySyncer(
writeNamespace: writeNamespace,
inputs: inputs,
mgr: mgr,
restCfg: restCfg,
k8sGwExtensions: k8sGwExtensions,
proxyTranslator: NewProxyTranslator(translator, xdsCache, settings, syncerExtensions, glooReporter),
istioClient: client,
Expand Down Expand Up @@ -189,12 +193,15 @@ type RedactedSecret krtcollections.ResourceWrapper[*gloov1.Secret]
func (us RedactedSecret) ResourceName() string {
return (krtcollections.ResourceWrapper[*gloov1.Secret](us)).ResourceName()
}

func (us RedactedSecret) Equals(in RedactedSecret) bool {
return proto.Equal(us.Inner, in.Inner)
}

var _ krt.ResourceNamer = RedactedSecret{}
var _ krt.Equaler[RedactedSecret] = RedactedSecret{}
var (
_ krt.ResourceNamer = RedactedSecret{}
_ krt.Equaler[RedactedSecret] = RedactedSecret{}
)

func (l RedactedSecret) MarshalJSON() ([]byte, error) {
return json.Marshal(struct {
Expand Down Expand Up @@ -298,7 +305,8 @@ func (s *ProxySyncer) Init(ctx context.Context, dbg *krt.DebugHandler) error {
// Annotation is never used - and may cause jitter as these are used for leader election.
t.GetObjectMeta().SetAnnotations(nil)
return obj, nil
}})
},
})
configMaps := krt.WrapClient(configMapClient, krt.WithName("ConfigMaps"), withDebug)

secretClient := kclient.New[*corev1.Secret](s.istioClient)
Expand Down Expand Up @@ -424,7 +432,7 @@ func (s *ProxySyncer) Init(ctx context.Context, dbg *krt.DebugHandler) error {
}, withDebug, krt.WithName("MostXdsSnapshots"))

if s.initialSettings.Spec.GetGloo().GetIstioOptions().GetEnableIntegration().GetValue() {
s.destRules = NewDestRuleIndex(s.istioClient, dbg)
s.destRules = NewDestRuleIndex(ctx, s.restCfg, s.istioClient, dbg)
} else {
s.destRules = NewEmptyDestRuleIndex()
}
Expand Down
48 changes: 37 additions & 11 deletions projects/gateway2/setup/ggv2setup_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,15 +116,39 @@ func init() {
}

func TestScenarios(t *testing.T) {
proxy_syncer.UseDetailedUnmarshalling = true
writer.set(t)
os.Setenv("POD_NAMESPACE", "gwtest")
testEnv := &envtest.Environment{
CRDDirectoryPaths: []string{
t.Run("latest Istio CRDs", func(t *testing.T) {
testScenariosWithCRDs(t, []string{
filepath.Join("..", "crds"),
filepath.Join("..", "..", "..", "install", "helm", "gloo", "crds"),
filepath.Join("testdata", "istiocrds"),
},
)
})

t.Run("old Istio CRDs", func(t *testing.T) {
testScenariosWithCRDs(t, []string{
filepath.Join("..", "crds"),
filepath.Join("..", "..", "..", "install", "helm", "gloo", "crds"),
filepath.Join("testdata", "oldistiocrds"),
},
// these cases rely on DestinationRule v1, we only have v1beta1 here
"failover.yaml",
"failover-default.yaml",
"failover-default-diffns.yaml",
)
})
}

func testScenariosWithCRDs(
t *testing.T,
crdDirPaths []string,
skipScenarios ...string,
) {
proxy_syncer.UseDetailedUnmarshalling = true
writer.set(t)
os.Setenv("POD_NAMESPACE", "gwtest")
testEnv := &envtest.Environment{
CRDDirectoryPaths: crdDirPaths,
ErrorIfCRDPathMissing: true,
// set assets dir so we can run without the makefile
BinaryAssetsDirectory: getAssetsDir(t),
Expand Down Expand Up @@ -229,6 +253,9 @@ func TestScenarios(t *testing.T) {
if strings.HasSuffix(f.Name(), ".yaml") && !strings.HasSuffix(f.Name(), "-out.yaml") {
fullpath := filepath.Join("testdata", f.Name())
t.Run(strings.TrimSuffix(f.Name(), ".yaml"), func(t *testing.T) {
if slices.Contains(skipScenarios, f.Name()) {
t.Skip()
}
writer.set(t)
t.Cleanup(func() {
writer.set(parentT)
Expand All @@ -239,12 +266,11 @@ func TestScenarios(t *testing.T) {
t.Logf("krt state for failed test: %s %s", t.Name(), string(j))
}
})
//sadly tests can't run yet in parallel, as ggv2 will add all the k8s services as clusters. this means
// sadly tests can't run yet in parallel, as ggv2 will add all the k8s services as clusters. this means
// that we get test pollution.
// once we change it to only include the ones in the proxy, we can re-enable this
// t.Parallel()
testScenario(t, ctx, setupOpts.KrtDebugger, snapCache, client, xdsPort, fullpath)

})
}
}
Expand Down Expand Up @@ -281,7 +307,7 @@ func testScenario(t *testing.T, ctx context.Context, kdbg *krt.DebugHandler, sna
testyaml := strings.ReplaceAll(string(testyamlbytes), gwname, testgwname)

yamlfile := filepath.Join(t.TempDir(), "test.yaml")
os.WriteFile(yamlfile, []byte(testyaml), 0644)
os.WriteFile(yamlfile, []byte(testyaml), 0o644)

err = client.ApplyYAMLFiles("", yamlfile)
defer func() {
Expand Down Expand Up @@ -316,7 +342,7 @@ func testScenario(t *testing.T, ctx context.Context, kdbg *krt.DebugHandler, sna
if err != nil {
t.Fatalf("failed to serialize xdsDump: %v", err)
}
os.WriteFile(fout, d, 0644)
os.WriteFile(fout, d, 0o644)
t.Fatal("wrote out file - nothing to test")
}
dump.Compare(t, expectedXdsDump)
Expand Down Expand Up @@ -356,7 +382,8 @@ func newXdsDumper(t *testing.T, ctx context.Context, xdsPort int, gwname string)
dr: &discovery_v3.DiscoveryRequest{Node: &envoycore.Node{
Id: "gateway.gwtest",
Metadata: &structpb.Struct{
Fields: map[string]*structpb.Value{"role": {Kind: &structpb.Value_StringValue{StringValue: fmt.Sprintf("gloo-kube-gateway-api~%s~%s-%s", "gwtest", "gwtest", gwname)}}}},
Fields: map[string]*structpb.Value{"role": {Kind: &structpb.Value_StringValue{StringValue: fmt.Sprintf("gloo-kube-gateway-api~%s~%s-%s", "gwtest", "gwtest", gwname)}}},
},
}},
}

Expand All @@ -373,7 +400,6 @@ func newXdsDumper(t *testing.T, ctx context.Context, xdsPort int, gwname string)
}

func (x xdsDumper) Dump(t *testing.T, ctx context.Context) xdsDump {

dr := proto.Clone(x.dr).(*discovery_v3.DiscoveryRequest)
dr.TypeUrl = "type.googleapis.com/envoy.config.cluster.v3.Cluster"
x.adsClient.Send(dr)
Expand Down
Loading

0 comments on commit 522171f

Please sign in to comment.