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.

Sign up for GitHub

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

Jump to bottom

sds: simplify implementation to reduce dependencies #50828

Merged
merged 9 commits into from
May 13, 2024
Merged

sds: simplify implementation to reduce dependencies #50828

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
6566b8a
wip
kyessenov May 2, 2024
b58884a
add check
kyessenov May 2, 2024
a0ebf43
sds: simplify implementation to reduce the dependencies
kyessenov May 2, 2024
59b0b6b
debug piggyback
kyessenov May 2, 2024
07bebb2
Merge remote-tracking branch 'upstream/master' into xds_sds
kyessenov May 7, 2024
7ea0357
disable piggyback
kyessenov May 7, 2024
c96c08a
Merge remote-tracking branch 'upstream/master' into xds_sds
kyessenov May 8, 2024
2fb2a06
merge fix
kyessenov May 13, 2024
603ac31
merge fix
kyessenov May 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 4 additions & 2 deletions Makefile.core.mk
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
xdsserver "istio.io/istio/pkg/xds"
"istio.io/istio/pkg/xds"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Package name conflict.

)

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
View file
Open in desktop
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"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same, with pilot/test/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
View file
Open in desktop
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
View file
Open in desktop
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