Skip to content

Commit 6e909d2

Browse files
sheidkampdavidjumanisoloio-bulldozer[bot]ben-taussig-solo
authored
[V1.16] Secret Deletion Validation (#9207)
* Secret Deletion validation against Previous State (#9122) * poc * updated logging * Update validator_test.go * Update api_snapshot.sk.go * Update validator.go * build? * Update validator.go * Update validator.go * logging for ee * Update validator.go * updates and logging * tightening * reverting githook * Create secret-deletion-with-warnings.yaml * updates, tests passing * extra logging * Progress * separating errors and warnings in validation * generate and cleanup * Adding secret deletion test table * Update validator.go * Update aggregate_translator.go * Update validator_test.go * Update go.mod * Update go.mod * Revert "Update go.mod" This reverts commit 19d72b7. * unused artifact * generate * Adding changelog file to new location * Deleting changelog file from old location * Empty-Commit * change EnableValidationAgainstSnapshot to DisableValidationAgainstSnapshot * error tests * fixing env * generate * Pr feedback * Apply suggestions from code review Co-authored-by: David Jumani <[email protected]> * ordering proxies and PR feedback * Added gloo validation scenarios * Test updates * Update validator.go * Adding changelog file to new location * Deleting changelog file from old location * comments * Update validator.go * cleanup * (feature branch target) Sheidkamp/secret delete no warnings (#9161) * cleanup * Update validator.go Stop collecting separate warnings * pull in modified solo-kit and update comments * remove irrelevant `DO_NOT_SUBMITS` * generate * Add more errors to breaking error validation * Update projects/gateway/pkg/validation/validator.go Co-authored-by: Ben Taussig <[email protected]> * PR Feedback * Add kube2e test * Update gateway_test.go * Revert "Update gateway_test.go" This reverts commit e83ab25. * Update gateway_test.go * kube e2e tests * Update validator.go * Update gateway_test.go * Update solo-kit and generate * typos * Pr feedback * Update test/kube2e/gateway/gateway_test.go Co-authored-by: David Jumani <[email protected]> * cleanup * Rename `AgainstSnapshot`s to `AgainstPreviousState` * generate --------- Co-authored-by: changelog-bot <changelog-bot> Co-authored-by: David Jumani <[email protected]> Co-authored-by: soloio-bulldozer[bot] <48420018+soloio-bulldozer[bot]@users.noreply.github.com> Co-authored-by: Ben Taussig <[email protected]> * generate * fix tests * disableKubernetesDestinations: false * disable by default * No 'NEW_FEATURES' in backport --------- Co-authored-by: David Jumani <[email protected]> Co-authored-by: soloio-bulldozer[bot] <48420018+soloio-bulldozer[bot]@users.noreply.github.com> Co-authored-by: Ben Taussig <[email protected]>
1 parent 7575d96 commit 6e909d2

File tree

15 files changed

+1158
-218
lines changed

15 files changed

+1158
-218
lines changed

‎changelog/v1.16.5/secret-delete-with-revalidation.yaml

+29
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
changelog:
2+
- type: FIX
3+
issueLink: https://github.com/solo-io/gloo/issues/8931
4+
resolvesIssue: false
5+
description: >
6+
Update to allow deletion of secrets when warnings or errors are present.
7+
8+
When the deletion of a secret is validated, the validating admission webhook removes the secret from the current snapshot,
9+
runs translations and looks for errors. Previously, the secret would not be deleted if there were errors, or if there
10+
were warnings and the `ignore_warnings` setting was set as `false`. This casues issues when trying to delete secrets that
11+
are unrelated to the warnings or errors.
12+
13+
The new behavior is to collect all the artifacts of the valdiation process, rerun validation against the original snapshot,
14+
and compare the artifacts from that process to the artifacts previously collected. If the artifacts are the same, the secret
15+
did not degrade the system and the deletion is allowed. If the artifacts are different, the secret is not deleted and errors are returned.
16+
17+
Because this is a backport, the new behavior is disabled by default in order not to alter existing behavior. This feature can be
18+
turned on by setting the `DISABLE_VALIDATION_AGAINST_PREVIOUS_STATE` environment variable to `false` in the `gloo` deployment.
19+
A dedicated helm value has not been added, and the environment variable can be set using `gloo.gloo.deployment.customEnv`
20+
- type: DEPENDENCY_BUMP
21+
dependencyOwner: rotisserie
22+
dependencyRepo: eris
23+
dependencyTag: v0.5.4
24+
description: Upgrade to get 'As' functionality. No other notable changes.
25+
- type: DEPENDENCY_BUMP
26+
dependencyOwner: solo-io
27+
dependencyRepo: solo-kit
28+
dependencyTag: v0.34.2
29+
description: Pull in changes to guarantee ordering of validation errors and warnings

‎docs/content/static/content/osa_provided.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ Name|Version|License
3535
[onsi/gomega](https://github.com/onsi/gomega)|v1.27.10|MIT License
3636
[pkg/browser](https://github.com/pkg/browser)|v0.0.0-20180916011732-0a3d74bf9ce4|BSD 2-clause "Simplified" License
3737
[pkg/errors](https://github.com/pkg/errors)|v0.9.1|BSD 2-clause "Simplified" License
38-
[rotisserie/eris](https://github.com/rotisserie/eris)|v0.4.0|MIT License
38+
[rotisserie/eris](https://github.com/rotisserie/eris)|v0.5.4|MIT License
3939
[saiskee/gettercheck](https://github.com/saiskee/gettercheck)|v0.0.0-20210820204958-38443d06ebe0|MIT License
4040
[sergi/go-diff](https://github.com/sergi/go-diff)|v1.1.0|MIT License
4141
[spf13/afero](https://github.com/spf13/afero)|v1.9.2|Apache License 2.0

‎go.mod

+2-2
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ require (
4545
github.com/onsi/gomega v1.27.10
4646
github.com/pkg/browser v0.0.0-20180916011732-0a3d74bf9ce4
4747
github.com/pkg/errors v0.9.1
48-
github.com/rotisserie/eris v0.4.0
48+
github.com/rotisserie/eris v0.5.4
4949
github.com/saiskee/gettercheck v0.0.0-20210820204958-38443d06ebe0
5050
github.com/sergi/go-diff v1.1.0
5151
github.com/solo-io/go-list-licenses v0.1.4
@@ -57,7 +57,7 @@ require (
5757

5858
// Pinned to the latest `gloo-repo-branch` tag of solo-apis (`sa-k8s-1.28-bump`)
5959
github.com/solo-io/solo-apis v0.0.0-20231206142556-d2e3ed6d4476
60-
github.com/solo-io/solo-kit v0.34.0
60+
github.com/solo-io/solo-kit v0.34.2
6161
github.com/spf13/afero v1.9.2
6262
github.com/spf13/cobra v1.7.0
6363
github.com/spf13/pflag v1.0.5

‎go.sum

+4-4
Original file line numberDiff line numberDiff line change
@@ -1748,8 +1748,8 @@ github.com/rogpeppe/go-internal v1.11.0 h1:cWPaGQEPrBb5/AsnsZesgZZ9yb1OQ+GOISoDN
17481748
github.com/rogpeppe/go-internal v1.11.0/go.mod h1:ddIwULY96R17DhadqLgMfk9H9tvdUzkipdSkR5nkCZA=
17491749
github.com/rollbar/rollbar-go v1.0.2/go.mod h1:AcFs5f0I+c71bpHlXNNDbOWJiKwjFDtISeXco0L5PKQ=
17501750
github.com/rotisserie/eris v0.1.1/go.mod h1:2ik3CyJrzlOjGyDGrKfqZivSfmkhCS3ktE+T1mNzzLk=
1751-
github.com/rotisserie/eris v0.4.0 h1:wfZW5hp90Y386s54DoJDK2Th3ycZotiBGM7b5b5aIHI=
1752-
github.com/rotisserie/eris v0.4.0/go.mod h1:lODN/gtqebxPHRbCcWeCYOE350FC2M3V/oAPT2wKxAU=
1751+
github.com/rotisserie/eris v0.5.4 h1:Il6IvLdAapsMhvuOahHWiBnl1G++Q0/L5UIkI5mARSk=
1752+
github.com/rotisserie/eris v0.5.4/go.mod h1:Z/kgYTJiJtocxCbFfvRmO+QejApzG6zpyky9G1A4g9s=
17531753
github.com/rs/dnscache v0.0.0-20211102005908-e0241e321417/go.mod h1:qe5TWALJ8/a1Lqznoc5BDHpYX/8HU60Hm2AwRmqzxqA=
17541754
github.com/rs/xid v1.2.1/go.mod h1:+uKXf+4Djp6Md1KODXJxgGQPKngRmWyn10oCKFzNHOQ=
17551755
github.com/rs/zerolog v1.9.1/go.mod h1:YbFCdg8HfsridGWAh22vktObvhZbQsZXe4/zB0OKkWU=
@@ -1815,8 +1815,8 @@ github.com/solo-io/skv2 v0.36.0 h1:Z1iCQou0Ei5K97UShAmB8xPYrFFZeseMZvef+kZ4Of0=
18151815
github.com/solo-io/skv2 v0.36.0/go.mod h1:0GKILLOrQiTKvGsf7UEFZJiDxJHrp0yKJQ6NbHzn+vY=
18161816
github.com/solo-io/solo-apis v0.0.0-20231206142556-d2e3ed6d4476 h1:qtImo1deMtbVHKKkepwKfu1CDJBjGPvTmADtXVWBynE=
18171817
github.com/solo-io/solo-apis v0.0.0-20231206142556-d2e3ed6d4476/go.mod h1:/2NUdNZ37KAhD1AfwFGR54rOK7ldDvmro17c0YRriKk=
1818-
github.com/solo-io/solo-kit v0.34.0 h1:TbcxhR7qBr5UVjVG5QfoGawVdrEqBG14DLrgLmePfmo=
1819-
github.com/solo-io/solo-kit v0.34.0/go.mod h1:HkrxDRo+uR0FZUJi+SGAUkbrUNWC6V0FD7GGXbtC0UU=
1818+
github.com/solo-io/solo-kit v0.34.2 h1:qcQGvhGEBDY5zvDMcP7z4BjAi+v66BtNLm984hZQK/s=
1819+
github.com/solo-io/solo-kit v0.34.2/go.mod h1:HkrxDRo+uR0FZUJi+SGAUkbrUNWC6V0FD7GGXbtC0UU=
18201820
github.com/spaolacci/murmur3 v0.0.0-20180118202830-f09979ecbc72/go.mod h1:JwIasOWyU6f++ZhiEuf87xNszmSA2myDM2Kzu9HwQUA=
18211821
github.com/spf13/afero v1.1.2/go.mod h1:j4pytiNVoe2o6bmDsKpLACNPDBIoEAkihy7loJ1B0CQ=
18221822
github.com/spf13/afero v1.2.2/go.mod h1:9ZxEEn6pIJ8Rxe320qSDBk6AsU0r9pR7Q4OcevTdifk=

‎projects/gateway/pkg/utils/gateway_filtering.go

+37
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
package utils
22

33
import (
4+
"sort"
5+
46
"github.com/solo-io/gloo/projects/gateway/pkg/defaults"
57

68
v1 "github.com/solo-io/gloo/projects/gateway/pkg/api/v1"
@@ -17,6 +19,41 @@ func GatewaysByProxyName(gateways v1.GatewayList) map[string]v1.GatewayList {
1719
return result
1820
}
1921

22+
// GatewaysAndProxyName is a struct that holds a list of gateways and a proxy name
23+
type GatewaysAndProxyName struct {
24+
Gateways v1.GatewayList
25+
Name string
26+
}
27+
28+
// SortedGatewaysByProxyName returns a slice of GatewaysAndProxyName structs, sorted by proxy name
29+
func SortedGatewaysByProxyName(gateways v1.GatewayList) []GatewaysAndProxyName {
30+
// Create the mapping
31+
gatewaysByProxyName := make(map[string]v1.GatewayList)
32+
for _, gw := range gateways {
33+
proxyNames := GetProxyNamesForGateway(gw)
34+
for _, name := range proxyNames {
35+
gatewaysByProxyName[name] = append(gatewaysByProxyName[name], gw)
36+
}
37+
}
38+
39+
// Create and populate the slice
40+
var gatewayAndProxyNames []GatewaysAndProxyName
41+
42+
for name, gateways := range gatewaysByProxyName {
43+
gatewayAndProxyNames = append(gatewayAndProxyNames, GatewaysAndProxyName{
44+
Gateways: gateways,
45+
Name: name,
46+
})
47+
}
48+
49+
// sort the slice by the struct's Name field
50+
sort.Slice(gatewayAndProxyNames, func(i, j int) bool {
51+
return gatewayAndProxyNames[i].Name < gatewayAndProxyNames[j].Name
52+
})
53+
54+
return gatewayAndProxyNames
55+
}
56+
2057
func GetProxyNamesForGateway(gw *v1.Gateway) []string {
2158
proxyNames := gw.GetProxyNames()
2259
if len(proxyNames) == 0 {

‎projects/gateway/pkg/utils/gateway_filtering_test.go

+25
Original file line numberDiff line numberDiff line change
@@ -30,4 +30,29 @@ var _ = Describe("gateway util unit tests", func() {
3030
}))
3131
})
3232
})
33+
34+
Describe("SortedGatewaysByProxyName", func() {
35+
// Must pass repeatedly so we don't accidentally get the right order by chance
36+
It("assigns each gateway once to each proxy by their proxyNames", MustPassRepeatedly(5), func() {
37+
38+
gws := v1.GatewayList{
39+
{Metadata: &core.Metadata{Name: "gw5"}, ProxyNames: []string{"proxy3", "proxy2"}},
40+
{Metadata: &core.Metadata{Name: "gw4"}, ProxyNames: []string{"proxy1", "proxy4"}},
41+
{Metadata: &core.Metadata{Name: "gw3"}, ProxyNames: []string{"proxy1", defaults.GatewayProxyName}},
42+
{Metadata: &core.Metadata{Name: "gw2"}, ProxyNames: []string{"proxy1", "proxy2"}},
43+
{Metadata: &core.Metadata{Name: "gw1"}, ProxyNames: nil /*default proxy*/},
44+
}
45+
46+
gw5, gw4, gw3, gw2, gw1 := gws[0], gws[1], gws[2], gws[3], gws[4]
47+
48+
byProxy := SortedGatewaysByProxyName(gws)
49+
Expect(byProxy).To(Equal([]GatewaysAndProxyName{
50+
{Gateways: v1.GatewayList{gw3, gw1}, Name: defaults.GatewayProxyName},
51+
{Gateways: v1.GatewayList{gw4, gw3, gw2}, Name: "proxy1"},
52+
{Gateways: v1.GatewayList{gw5, gw2}, Name: "proxy2"},
53+
{Gateways: v1.GatewayList{gw5}, Name: "proxy3"},
54+
{Gateways: v1.GatewayList{gw4}, Name: "proxy4"},
55+
}))
56+
})
57+
})
3358
})

0 commit comments

Comments
 (0)