Skip to content

Commit

Permalink
Merge pull request #123770 from Jefftree/go-restful
Browse files Browse the repository at this point in the history 
fix aggregator path filtering to include /

Kubernetes-commit: 46f017a90b54b5abfc0a66a6ecf19e304aa7da95
  • Loading branch information
@k8s-publishing-bot
k8s-publishing-bot committed Mar 27, 2024
2 parents 0484f16 + 0ff135f commit 03409cd
Show file tree
Hide file tree
Showing 4 changed files with 65 additions and 73 deletions.
22 changes: 6 additions & 16 deletions go.mod
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,11 @@ require (
github.com/spf13/pflag v1.0.5
github.com/stretchr/testify v1.8.4
golang.org/x/net v0.21.0
k8s.io/api v0.0.0-20240306165540-05aa4bceed70
k8s.io/apimachinery v0.0.0-20240306164812-cbfe0a1feaa5
k8s.io/apiserver v0.0.0-20240306172940-17663913a4fd
k8s.io/client-go v0.0.0-20240306170515-0cdc0ce850af
k8s.io/code-generator v0.0.0-20240306171749-c9df80eb5e54
k8s.io/api v0.0.0-20240307045517-1d33fcde6f3f
k8s.io/apimachinery v0.0.0-20240307160843-0407311be590
k8s.io/apiserver v0.0.0-20240307053455-8763b7fa93af
k8s.io/client-go v0.0.0-20240307050516-47abbe025115
k8s.io/code-generator v0.0.0-20240327163807-71421176fc0e
k8s.io/component-base v0.0.0-20240306172020-b0a6e40497ae
k8s.io/klog/v2 v2.120.1
k8s.io/kube-openapi v0.0.0-20240228011516-70dd3763d340
Expand Down Expand Up @@ -105,18 +105,8 @@ require (
gopkg.in/yaml.v2 v2.4.0 // indirect
gopkg.in/yaml.v3 v3.0.1 // indirect
k8s.io/gengo/v2 v2.0.0-20240228010128-51d4e06bde70 // indirect
k8s.io/kms v0.0.0-20240306172340-03c9a46c21c7 // indirect
k8s.io/kms v0.0.0-20240327165525-6bf80055228c // indirect
sigs.k8s.io/apiserver-network-proxy/konnectivity-client v0.29.0 // indirect
sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd // indirect
sigs.k8s.io/yaml v1.3.0 // indirect
)

replace (
k8s.io/api => k8s.io/api v0.0.0-20240306165540-05aa4bceed70
k8s.io/apimachinery => k8s.io/apimachinery v0.0.0-20240306164812-cbfe0a1feaa5
k8s.io/apiserver => k8s.io/apiserver v0.0.0-20240306172940-17663913a4fd
k8s.io/client-go => k8s.io/client-go v0.0.0-20240306170515-0cdc0ce850af
k8s.io/code-generator => k8s.io/code-generator v0.0.0-20240306171749-c9df80eb5e54
k8s.io/component-base => k8s.io/component-base v0.0.0-20240306172020-b0a6e40497ae
k8s.io/kms => k8s.io/kms v0.0.0-20240306172340-03c9a46c21c7
)
24 changes: 12 additions & 12 deletions go.sum
View file
Original file line number Diff line number Diff line change
Expand Up @@ -292,24 +292,24 @@ gopkg.in/yaml.v2 v2.4.0/go.mod h1:RDklbk79AGWmwhnvt/jBztapEOGDOx6ZbXqjP6csGnQ=
gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA=
gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
k8s.io/api v0.0.0-20240306165540-05aa4bceed70 h1:Kw6GufAvqr668v56lckDWoGHFG5wwjQRjYuf+PMt1us=
k8s.io/api v0.0.0-20240306165540-05aa4bceed70/go.mod h1:S69aw/5045kbDeBVmy89EQxSM7v5kHdLNzO4wt3OZ2o=
k8s.io/apimachinery v0.0.0-20240306164812-cbfe0a1feaa5 h1:YRP8FbAab9hlobsEfyUG7P6dC6hbVTLw0eFY/AnewmY=
k8s.io/apimachinery v0.0.0-20240306164812-cbfe0a1feaa5/go.mod h1:wEJvNDlfxMRaMhyv38SIHIEC9hah/xuzqUUhxIyUv7Y=
k8s.io/apiserver v0.0.0-20240306172940-17663913a4fd h1:epNppmSMUuJL2TpEKJN9mGdipws7IAsRp4wHozRLnmI=
k8s.io/apiserver v0.0.0-20240306172940-17663913a4fd/go.mod h1:St1PmKeeAHl34HIFGJMPbD+sNLyXk4bRNv1ptaHPOg8=
k8s.io/client-go v0.0.0-20240306170515-0cdc0ce850af h1:hU8WcNO9vZrUXLzKbyXj6FUlMuTecjBr7MmbGxfCkzM=
k8s.io/client-go v0.0.0-20240306170515-0cdc0ce850af/go.mod h1:g+S/ljjD+b0SS7OkKeZ6IBio03Ot9Q8s19hN3jHsl/Y=
k8s.io/code-generator v0.0.0-20240306171749-c9df80eb5e54 h1:BooyL7i5YIYtXh4lUUDJ7d8qaFy/PFf+WvRSOK3UCrE=
k8s.io/code-generator v0.0.0-20240306171749-c9df80eb5e54/go.mod h1:gmJp+Ea+u8C4QlTlnkVtViOIsQKlpDCtC6zjiJaGCrc=
k8s.io/api v0.0.0-20240307045517-1d33fcde6f3f h1:AZBD10PF/cNxYJJgh8fMrENM3UItvPkMO18LB0dTrV8=
k8s.io/api v0.0.0-20240307045517-1d33fcde6f3f/go.mod h1:qUUt5cHNc6u/2o6gcfyGSFTAK8Ca4QD40QqESHedUoc=
k8s.io/apimachinery v0.0.0-20240307160843-0407311be590 h1:YFg0j+PVfNLayHtZ3gdTeW12q7HECwhvZm9fWZpXyXo=
k8s.io/apimachinery v0.0.0-20240307160843-0407311be590/go.mod h1:wEJvNDlfxMRaMhyv38SIHIEC9hah/xuzqUUhxIyUv7Y=
k8s.io/apiserver v0.0.0-20240307053455-8763b7fa93af h1:t14mpLj8FMKgkyGchVjIIq7RwowDSPuxw5os6m5hc00=
k8s.io/apiserver v0.0.0-20240307053455-8763b7fa93af/go.mod h1:HmmYDBhRoIbxko8OQcZG4K/ZPSY5cAcH1+c10kAKSyo=
k8s.io/client-go v0.0.0-20240307050516-47abbe025115 h1:5Dke+PgLxm5vHD81IKNgBvtlHJtU/lCE2S/ogshkics=
k8s.io/client-go v0.0.0-20240307050516-47abbe025115/go.mod h1:FOGFrZsYXNQPOpNFNqdr723jdHoEbjA71a1HGcj+2s8=
k8s.io/code-generator v0.0.0-20240327163807-71421176fc0e h1:zlsfzK9qpxCp2ahp494MTOUM0uxV8X5YaG4hfbtbcXI=
k8s.io/code-generator v0.0.0-20240327163807-71421176fc0e/go.mod h1:0VAoUbzMRwLcq5yON5v0Dv5ZX4OlSKp1Lj02Opwv4iA=
k8s.io/component-base v0.0.0-20240306172020-b0a6e40497ae h1:/bnYoyvHvOnxGRpKc3z/RG0R/gzUwIFg2zKk8UkVchM=
k8s.io/component-base v0.0.0-20240306172020-b0a6e40497ae/go.mod h1:xHRknJm8Q13BCZjYtVCjnJJEzKC9RMj6OqPgtPWU8yI=
k8s.io/gengo/v2 v2.0.0-20240228010128-51d4e06bde70 h1:NGrVE502P0s0/1hudf8zjgwki1X/TByhmAoILTarmzo=
k8s.io/gengo/v2 v2.0.0-20240228010128-51d4e06bde70/go.mod h1:VH3AT8AaQOqiGjMF9p0/IM1Dj+82ZwjfxUP1IxaHE+8=
k8s.io/klog/v2 v2.120.1 h1:QXU6cPEOIslTGvZaXvFWiP9VKyeet3sawzTOvdXb4Vw=
k8s.io/klog/v2 v2.120.1/go.mod h1:3Jpz1GvMt720eyJH1ckRHK1EDfpxISzJ7I9OYgaDtPE=
k8s.io/kms v0.0.0-20240306172340-03c9a46c21c7 h1:v9feiZ9mQnGuvoKR8FkDM/2yuptz78hIiFIGJrhY5oA=
k8s.io/kms v0.0.0-20240306172340-03c9a46c21c7/go.mod h1:Z8nrOW3UFa7WlN70KZvdbXOibJlhHr8pXAoqfjO/Owk=
k8s.io/kms v0.0.0-20240327165525-6bf80055228c h1:3xRskaoHe8gjLIIeH+2V800bl6ZluP67+88BlneBF0w=
k8s.io/kms v0.0.0-20240327165525-6bf80055228c/go.mod h1:Z8nrOW3UFa7WlN70KZvdbXOibJlhHr8pXAoqfjO/Owk=
k8s.io/kube-openapi v0.0.0-20240228011516-70dd3763d340 h1:BZqlfIlq5YbRMFko6/PM7FjZpUb45WallggurYhKGag=
k8s.io/kube-openapi v0.0.0-20240228011516-70dd3763d340/go.mod h1:yD4MZYeKMBwQKVht279WycxKyM84kkAx2DPrTXaeb98=
k8s.io/utils v0.0.0-20230726121419-3b25d923346b h1:sgn3ZU783SCgtaSJjpcVVlRqd6GSnlTLKgpAAttJvpI=
Expand Down
2 changes: 1 addition & 1 deletion pkg/controllers/openapi/aggregator/aggregator.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ func (s *specAggregator) updateServiceLocked(name string) error {
}
group := specInfo.apiService.Spec.Group
version := specInfo.apiService.Spec.Version
return aggregator.FilterSpecByPathsWithoutSideEffects(result, []string{"/apis/" + group + "/" + version}), etag, nil
return aggregator.FilterSpecByPathsWithoutSideEffects(result, []string{"/apis/" + group + "/" + version + "/"}), etag, nil
}, cached.Result[*spec.Swagger]{Value: result, Etag: etag, Err: err})
specInfo.spec.Store(filteredResult)
return err
Expand Down
90 changes: 46 additions & 44 deletions pkg/controllers/openapi/aggregator/aggregator_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ func TestBasicPathsMerged(t *testing.T) {
SwaggerProps: spec.SwaggerProps{
Paths: &spec.Paths{
Paths: map[string]spec.PathItem{
"/apis/foo/v1": {},
"/apis/foo/v1/": {},
},
},
},
Expand All @@ -52,8 +52,8 @@ func TestBasicPathsMerged(t *testing.T) {
if err != nil {
t.Error(err)
}
expectPath(t, swagger, "/apis/foo/v1")
expectPath(t, swagger, "/apis/apiregistration.k8s.io/v1")
expectPath(t, swagger, "/apis/foo/v1/")
expectPath(t, swagger, "/apis/apiregistration.k8s.io/v1/")
}

func TestAddUpdateAPIService(t *testing.T) {
Expand Down Expand Up @@ -103,7 +103,7 @@ func TestAddUpdateAPIService(t *testing.T) {
}

expectPath(t, swagger, "/apis/apiservicegroup/v1/path1")
expectPath(t, swagger, "/apis/apiregistration.k8s.io/v1")
expectPath(t, swagger, "/apis/apiregistration.k8s.io/v1/")

t.Log("Update APIService OpenAPI")
handler.openapi = &spec.Swagger{
Expand All @@ -127,7 +127,7 @@ func TestAddUpdateAPIService(t *testing.T) {
// aggregated OpenAPI is also updated.
expectPath(t, swagger, "/apis/apiservicegroup/v1/path2")
expectNoPath(t, swagger, "/apis/apiservicegroup/v1/path1")
expectPath(t, swagger, "/apis/apiregistration.k8s.io/v1")
expectPath(t, swagger, "/apis/apiregistration.k8s.io/v1/")
}

// Tests that an APIService that registers OpenAPI will only have the OpenAPI
Expand All @@ -140,7 +140,7 @@ func TestAPIServiceOpenAPIServiceMismatch(t *testing.T) {
SwaggerProps: spec.SwaggerProps{
Paths: &spec.Paths{
Paths: map[string]spec.PathItem{
"/apis/foo/v1": {},
"/apis/foo/v1/": {},
},
},
},
Expand Down Expand Up @@ -171,7 +171,8 @@ func TestAPIServiceOpenAPIServiceMismatch(t *testing.T) {
SwaggerProps: spec.SwaggerProps{
Paths: &spec.Paths{
Paths: map[string]spec.PathItem{
"/apis/apiservicegroup/v1": {},
"/apis/apiservicegroup/v1/": {},
"/apis/apiservicegroup/v1beta1/": {},
},
},
},
Expand All @@ -181,9 +182,9 @@ func TestAPIServiceOpenAPIServiceMismatch(t *testing.T) {
SwaggerProps: spec.SwaggerProps{
Paths: &spec.Paths{
Paths: map[string]spec.PathItem{
"/apis/a": {},
"/apis/apiservicegroup/v1": {},
"/apis/apiservicegroup/v2": {},
"/apis/a/": {},
"/apis/apiservicegroup/v1/": {},
"/apis/apiservicegroup/v2/": {},
},
},
},
Expand All @@ -207,10 +208,11 @@ func TestAPIServiceOpenAPIServiceMismatch(t *testing.T) {
if err != nil {
t.Error(err)
}
expectPath(t, swagger, "/apis/apiservicegroup/v1")
expectPath(t, swagger, "/apis/apiservicegroup/v2")
expectPath(t, swagger, "/apis/apiregistration.k8s.io/v1")
expectNoPath(t, swagger, "/apis/a")
expectPath(t, swagger, "/apis/apiservicegroup/v1/")
expectPath(t, swagger, "/apis/apiservicegroup/v2/")
expectPath(t, swagger, "/apis/apiregistration.k8s.io/v1/")
expectNoPath(t, swagger, "/apis/a/")
expectNoPath(t, swagger, "/apis/apiservicegroup/v1beta1/")

t.Logf("Remove APIService %s", apiService.Name)
s.RemoveAPIService(apiService.Name)
Expand All @@ -221,7 +223,7 @@ func TestAPIServiceOpenAPIServiceMismatch(t *testing.T) {
}
// Ensure that the if the APIService is added then removed, the OpenAPI disappears from the aggregated OpenAPI as well.
expectNoPath(t, swagger, "/apis/apiservicegroup/v1")
expectPath(t, swagger, "/apis/apiregistration.k8s.io/v1")
expectPath(t, swagger, "/apis/apiregistration.k8s.io/v1/")
expectNoPath(t, swagger, "/apis/a")
}

Expand All @@ -232,7 +234,7 @@ func TestAddRemoveAPIService(t *testing.T) {
SwaggerProps: spec.SwaggerProps{
Paths: &spec.Paths{
Paths: map[string]spec.PathItem{
"/apis/foo/v1": {},
"/apis/foo/v1/": {},
},
},
},
Expand All @@ -254,7 +256,7 @@ func TestAddRemoveAPIService(t *testing.T) {
SwaggerProps: spec.SwaggerProps{
Paths: &spec.Paths{
Paths: map[string]spec.PathItem{
"/apis/apiservicegroup/v1": {},
"/apis/apiservicegroup/v1/": {},
},
},
},
Expand All @@ -271,8 +273,8 @@ func TestAddRemoveAPIService(t *testing.T) {
if err != nil {
t.Error(err)
}
expectPath(t, swagger, "/apis/apiservicegroup/v1")
expectPath(t, swagger, "/apis/apiregistration.k8s.io/v1")
expectPath(t, swagger, "/apis/apiservicegroup/v1/")
expectPath(t, swagger, "/apis/apiregistration.k8s.io/v1/")

t.Logf("Remove APIService %s", apiService.Name)
s.RemoveAPIService(apiService.Name)
Expand All @@ -282,8 +284,8 @@ func TestAddRemoveAPIService(t *testing.T) {
t.Error(err)
}
// Ensure that the if the APIService is added then removed, the OpenAPI disappears from the aggregated OpenAPI as well.
expectNoPath(t, swagger, "/apis/apiservicegroup/v1")
expectPath(t, swagger, "/apis/apiregistration.k8s.io/v1")
expectNoPath(t, swagger, "/apis/apiservicegroup/v1/")
expectPath(t, swagger, "/apis/apiregistration.k8s.io/v1/")
}

func TestUpdateAPIService(t *testing.T) {
Expand All @@ -293,7 +295,7 @@ func TestUpdateAPIService(t *testing.T) {
SwaggerProps: spec.SwaggerProps{
Paths: &spec.Paths{
Paths: map[string]spec.PathItem{
"/apis/foo/v1": {},
"/apis/foo/v1/": {},
},
},
},
Expand All @@ -315,7 +317,7 @@ func TestUpdateAPIService(t *testing.T) {
SwaggerProps: spec.SwaggerProps{
Paths: &spec.Paths{
Paths: map[string]spec.PathItem{
"/apis/apiservicegroup/v1": {},
"/apis/apiservicegroup/v1/": {},
},
},
},
Expand All @@ -340,8 +342,8 @@ func TestUpdateAPIService(t *testing.T) {
if err != nil {
t.Error(err)
}
expectPath(t, swagger, "/apis/apiservicegroup/v1")
expectPath(t, swagger, "/apis/apiregistration.k8s.io/v1")
expectPath(t, swagger, "/apis/apiservicegroup/v1/")
expectPath(t, swagger, "/apis/apiregistration.k8s.io/v1/")

t.Logf("Updating APIService %s", apiService.Name)
if err := s.AddUpdateAPIService(apiService, handler2); err != nil {
Expand All @@ -356,8 +358,8 @@ func TestUpdateAPIService(t *testing.T) {
t.Error(err)
}
// Ensure that the if the APIService is added and then handler is modified, the new data is reflected in the aggregated OpenAPI.
expectNoPath(t, swagger, "/apis/apiservicegroup/v1")
expectPath(t, swagger, "/apis/apiregistration.k8s.io/v1")
expectNoPath(t, swagger, "/apis/apiservicegroup/v1/")
expectPath(t, swagger, "/apis/apiregistration.k8s.io/v1/")
}

func TestFailingAPIServiceSkippedAggregation(t *testing.T) {
Expand All @@ -367,7 +369,7 @@ func TestFailingAPIServiceSkippedAggregation(t *testing.T) {
SwaggerProps: spec.SwaggerProps{
Paths: &spec.Paths{
Paths: map[string]spec.PathItem{
"/apis/foo/v1": {},
"/apis/foo/v1/": {},
},
},
},
Expand All @@ -391,7 +393,7 @@ func TestFailingAPIServiceSkippedAggregation(t *testing.T) {
SwaggerProps: spec.SwaggerProps{
Paths: &spec.Paths{
Paths: map[string]spec.PathItem{
"/apis/failed/v1": {},
"/apis/failed/v1/": {},
},
},
},
Expand All @@ -412,7 +414,7 @@ func TestFailingAPIServiceSkippedAggregation(t *testing.T) {
SwaggerProps: spec.SwaggerProps{
Paths: &spec.Paths{
Paths: map[string]spec.PathItem{
"/apis/success/v1": {},
"/apis/success/v1/": {},
},
},
},
Expand All @@ -437,9 +439,9 @@ func TestFailingAPIServiceSkippedAggregation(t *testing.T) {
if err != nil {
t.Error(err)
}
expectPath(t, swagger, "/apis/foo/v1")
expectNoPath(t, swagger, "/apis/failed/v1")
expectPath(t, swagger, "/apis/success/v1")
expectPath(t, swagger, "/apis/foo/v1/")
expectNoPath(t, swagger, "/apis/failed/v1/")
expectPath(t, swagger, "/apis/success/v1/")
}

func TestAPIServiceFailSuccessTransition(t *testing.T) {
Expand All @@ -449,7 +451,7 @@ func TestAPIServiceFailSuccessTransition(t *testing.T) {
SwaggerProps: spec.SwaggerProps{
Paths: &spec.Paths{
Paths: map[string]spec.PathItem{
"/apis/foo/v1": {},
"/apis/foo/v1/": {},
},
},
},
Expand All @@ -473,7 +475,7 @@ func TestAPIServiceFailSuccessTransition(t *testing.T) {
SwaggerProps: spec.SwaggerProps{
Paths: &spec.Paths{
Paths: map[string]spec.PathItem{
"/apis/apiservicegroup/v1": {},
"/apis/apiservicegroup/v1/": {},
},
},
},
Expand All @@ -491,8 +493,8 @@ func TestAPIServiceFailSuccessTransition(t *testing.T) {
if err != nil {
t.Error(err)
}
expectPath(t, swagger, "/apis/foo/v1")
expectNoPath(t, swagger, "/apis/apiservicegroup/v1")
expectPath(t, swagger, "/apis/foo/v1/")
expectNoPath(t, swagger, "/apis/apiservicegroup/v1/")

t.Log("Transition APIService to not return error")
handler.returnErr = false
Expand All @@ -504,8 +506,8 @@ func TestAPIServiceFailSuccessTransition(t *testing.T) {
if err != nil {
t.Error(err)
}
expectPath(t, swagger, "/apis/foo/v1")
expectPath(t, swagger, "/apis/apiservicegroup/v1")
expectPath(t, swagger, "/apis/foo/v1/")
expectPath(t, swagger, "/apis/apiservicegroup/v1/")
}

func TestFailingAPIServiceDoesNotBlockAdd(t *testing.T) {
Expand All @@ -515,7 +517,7 @@ func TestFailingAPIServiceDoesNotBlockAdd(t *testing.T) {
SwaggerProps: spec.SwaggerProps{
Paths: &spec.Paths{
Paths: map[string]spec.PathItem{
"/apis/foo/v1": {},
"/apis/foo/v1/": {},
},
},
},
Expand All @@ -542,7 +544,7 @@ func TestFailingAPIServiceDoesNotBlockAdd(t *testing.T) {
SwaggerProps: spec.SwaggerProps{
Paths: &spec.Paths{
Paths: map[string]spec.PathItem{
"/apis/failed/v1": {},
"/apis/failed/v1/": {},
},
},
},
Expand All @@ -567,8 +569,8 @@ func TestFailingAPIServiceDoesNotBlockAdd(t *testing.T) {
if err != nil {
t.Error(err)
}
expectPath(t, swagger, "/apis/foo/v1")
expectNoPath(t, swagger, "/apis/failed/v1")
expectPath(t, swagger, "/apis/foo/v1/")
expectNoPath(t, swagger, "/apis/failed/v1/")
}

type openAPIHandler struct {
Expand Down Expand Up @@ -619,7 +621,7 @@ func buildAndRegisterSpecAggregator(delegationHandlers []http.Handler, mux commo
SwaggerProps: spec.SwaggerProps{
Paths: &spec.Paths{
Paths: map[string]spec.PathItem{
"/apis/apiregistration.k8s.io/v1": {},
"/apis/apiregistration.k8s.io/v1/": {},
},
},
},
Expand Down

0 comments on commit 03409cd

Please sign in to comment.