Skip to content

Commit

Permalink
sds: simplify implementation to reduce dependencies (#50828)
Browse files Browse the repository at this point in the history
* wip

Change-Id: Ie9e4dd8a0a404671616e2fd1b97ffbe558fa85d1
Signed-off-by: Kuat Yessenov <[email protected]>

* add check

Change-Id: I7b318d10d4809ea97693d587425bafbd22f53b6a
Signed-off-by: Kuat Yessenov <[email protected]>

* sds: simplify implementation to reduce the dependencies

Change-Id: Id8d118a96f38895a6e0e334a2d9a57645e9b53d6
Signed-off-by: Kuat Yessenov <[email protected]>

* debug piggyback

Change-Id: Ice126f4bcbbc364c511139cf21fe5e5f4991de51
Signed-off-by: Kuat Yessenov <[email protected]>

* disable piggyback

Change-Id: Ia02658149db8f3f5d663774de75733cb067a7250
Signed-off-by: Kuat Yessenov <[email protected]>

* merge fix

Change-Id: I084740c88ed006f593384d4f8eb0cf42a3200274
Signed-off-by: Kuat Yessenov <[email protected]>

---------

Signed-off-by: Kuat Yessenov <[email protected]>
  • Loading branch information
kyessenov committed May 13, 2024
1 parent e05f336 commit eacdbed
Show file tree
Hide file tree
Showing 10 changed files with 169 additions and 154 deletions.
6 changes: 4 additions & 2 deletions Makefile.core.mk
Original file line number Diff line number Diff line change
Expand Up @@ -290,17 +290,19 @@ lint: lint-python lint-copyright-banner lint-scripts lint-go lint-dockerfiles li
# (proto) Envoy TLS proto for SDS
# (proto) Envoy Wasm filters for wasm xDS proxy
# (proto) xDS discovery service for xDS proxy
# (proto) SDS secret and contrib QAT and cryptomb
.PHONY: check-agent-deps
check-agent-deps:
@go list -f '{{ join .Deps "\n" }}' -tags=agent \
./pilot/cmd/pilot-agent/app \
./pilot/cmd/pilot-agent/... \
./pkg/istio-agent/... | sort | uniq |\
grep -Pv '^k8s.io/(utils|klog|apimachinery)/' |\
grep -Pv 'envoy/type/|envoy/annotations|envoy/config/core/' |\
grep -Pv 'envoy/extensions/transport_sockets/tls/' |\
grep -Pv 'envoy/service/discovery/v3' |\
grep -Pv 'envoy/service/(discovery|secret)/v3' |\
grep -Pv 'envoy/extensions/wasm/' |\
grep -Pv 'envoy/extensions/filters/(http|network)/wasm/' |\
grep -Pv 'contrib/envoy/extensions/private_key_providers/' |\
grep -Pv 'istio\.io/api/(annotation|label|mcp|mesh|networking|security/v1alpha1|type)' |\
(! grep -P '^k8s.io|^sigs.k8s.io/gateway-api|cel|antlr|jwx/jwk|envoy/|istio.io/api')

Expand Down
8 changes: 0 additions & 8 deletions pilot/pkg/model/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -272,14 +272,6 @@ func AnyToUnnamedResources(r []*anypb.Any) Resources {
return a
}

func ResourcesToAny(r Resources) []*anypb.Any {
a := make([]*anypb.Any, 0, len(r))
for _, rr := range r {
a = append(a, rr.Resource)
}
return a
}

// XdsUpdates include information about the subset of updated resources.
// See for example EDS incremental updates.
type XdsUpdates = sets.Set[ConfigKey]
Expand Down
3 changes: 2 additions & 1 deletion pilot/pkg/xds/delta_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import (
"istio.io/istio/pkg/test/util/assert"
"istio.io/istio/pkg/util/sets"
"istio.io/istio/pkg/workloadapi"
xdsserver "istio.io/istio/pkg/xds"
)

func TestDeltaAds(t *testing.T) {
Expand All @@ -53,7 +54,7 @@ func TestDeltaAdsClusterUpdate(t *testing.T) {
ResourceNamesUnsubscribe: remove,
})
nonce = res.Nonce
got := xdstest.MapKeys(xdstest.ExtractLoadAssignments(xdstest.UnmarshalClusterLoadAssignment(t, model.ResourcesToAny(res.Resources))))
got := xdstest.MapKeys(xdstest.ExtractLoadAssignments(xdstest.UnmarshalClusterLoadAssignment(t, xdsserver.ResourcesToAny(res.Resources))))
if !reflect.DeepEqual(expect, got) {
t.Fatalf("expected clusters %v got %v", expect, got)
}
Expand Down
13 changes: 7 additions & 6 deletions pilot/pkg/xds/sds_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ import (
"istio.io/istio/pkg/spiffe"
"istio.io/istio/pkg/test/env"
"istio.io/istio/pkg/util/sets"
xdsserver "istio.io/istio/pkg/xds"
)

func makeSecret(name string, data map[string]string) *corev1.Secret {
Expand Down Expand Up @@ -329,7 +330,7 @@ func TestGenerateSDS(t *testing.T) {
gen := s.Discovery.Generators[v3.SecretType]
tt.request.Start = time.Now()
secrets, _, _ := gen.Generate(s.SetupProxy(tt.proxy), &model.WatchedResource{ResourceNames: tt.resources}, tt.request)
raw := xdstest.ExtractTLSSecrets(t, model.ResourcesToAny(secrets))
raw := xdstest.ExtractTLSSecrets(t, xdsserver.ResourcesToAny(secrets))

got := map[string]Expected{}
for _, scrt := range raw {
Expand Down Expand Up @@ -375,14 +376,14 @@ func TestCaching(t *testing.T) {
}

secrets, _, _ := gen.Generate(s.SetupProxy(istiosystem), &model.WatchedResource{ResourceNames: []string{"kubernetes://generic"}}, fullPush)
raw := xdstest.ExtractTLSSecrets(t, model.ResourcesToAny(secrets))
raw := xdstest.ExtractTLSSecrets(t, xdsserver.ResourcesToAny(secrets))
if len(raw) != 1 {
t.Fatalf("failed to get expected secrets for authorized proxy: %v", raw)
}

// We should not get secret returned, even though we are asking for the same one
secrets, _, _ = gen.Generate(s.SetupProxy(otherNamespace), &model.WatchedResource{ResourceNames: []string{"kubernetes://generic"}}, fullPush)
raw = xdstest.ExtractTLSSecrets(t, model.ResourcesToAny(secrets))
raw = xdstest.ExtractTLSSecrets(t, xdsserver.ResourcesToAny(secrets))
if len(raw) != 0 {
t.Fatalf("failed to get expected secrets for unauthorized proxy: %v", raw)
}
Expand Down Expand Up @@ -423,7 +424,7 @@ func TestPrivateKeyProviderProxyConfig(t *testing.T) {
gen := s.Discovery.Generators[v3.SecretType]
fullPush := &model.PushRequest{Full: true, Start: time.Now()}
secrets, _, _ := gen.Generate(s.SetupProxy(rawProxy), &model.WatchedResource{ResourceNames: []string{"kubernetes://generic"}}, fullPush)
raw := xdstest.ExtractTLSSecrets(t, model.ResourcesToAny(secrets))
raw := xdstest.ExtractTLSSecrets(t, xdsserver.ResourcesToAny(secrets))
for _, scrt := range raw {
if scrt.GetTlsCertificate().GetPrivateKeyProvider() != nil {
t.Fatalf("expect no private key provider in secret")
Expand All @@ -432,7 +433,7 @@ func TestPrivateKeyProviderProxyConfig(t *testing.T) {

// add private key provider in proxy-config
secrets, _, _ = gen.Generate(s.SetupProxy(pkpProxy), &model.WatchedResource{ResourceNames: []string{"kubernetes://generic"}}, fullPush)
raw = xdstest.ExtractTLSSecrets(t, model.ResourcesToAny(secrets))
raw = xdstest.ExtractTLSSecrets(t, xdsserver.ResourcesToAny(secrets))
for _, scrt := range raw {
privateKeyProvider := scrt.GetTlsCertificate().GetPrivateKeyProvider()
if privateKeyProvider == nil {
Expand All @@ -445,7 +446,7 @@ func TestPrivateKeyProviderProxyConfig(t *testing.T) {

// erase private key provider in proxy-config
secrets, _, _ = gen.Generate(s.SetupProxy(rawProxy), &model.WatchedResource{ResourceNames: []string{"kubernetes://generic"}}, fullPush)
raw = xdstest.ExtractTLSSecrets(t, model.ResourcesToAny(secrets))
raw = xdstest.ExtractTLSSecrets(t, xdsserver.ResourcesToAny(secrets))
for _, scrt := range raw {
if scrt.GetTlsCertificate().GetPrivateKeyProvider() != nil {
t.Fatalf("expect no private key provider in secret")
Expand Down
2 changes: 1 addition & 1 deletion pilot/pkg/xds/xdsgen.go
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ func (s *DiscoveryServer) pushXds(con *Connection, w *model.WatchedResource, req
// TODO: send different version for incremental eds
VersionInfo: req.Push.PushVersion,
Nonce: nonce(req.Push.LedgerVersion),
Resources: model.ResourcesToAny(res),
Resources: xds.ResourcesToAny(res),
}

configSize := ResourceSize(res)
Expand Down
9 changes: 9 additions & 0 deletions pkg/xds/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
discovery "github.com/envoyproxy/go-control-plane/envoy/service/discovery/v3"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
"google.golang.org/protobuf/types/known/anypb"

"istio.io/istio/pilot/pkg/features"
istiogrpc "istio.io/istio/pilot/pkg/grpc"
Expand All @@ -45,6 +46,14 @@ func (rd ResourceDelta) IsEmpty() bool {

type Resources = []*discovery.Resource

func ResourcesToAny(r Resources) []*anypb.Any {
a := make([]*anypb.Any, 0, len(r))
for _, rr := range r {
a = append(a, rr.Resource)
}
return a
}

// WatchedResource tracks an active DiscoveryRequest subscription.
type WatchedResource struct {
// TypeUrl is copied from the DiscoveryRequest.TypeUrl that initiated watching this resource.
Expand Down

0 comments on commit eacdbed

Please sign in to comment.