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

fix(*): merging arrays in new policies #5084

Merged
merged 27 commits into from
Oct 18, 2022
Merged

fix(*): merging arrays in new policies #5084

merged 27 commits into from
Oct 18, 2022

Conversation

lobkovilya
Copy link
Contributor

@lobkovilya lobkovilya commented Sep 30, 2022 •
edited by lukidzi
Loading

  • Encode new policies in KDS using encoding/json instead of jsonpb
  • Merge confs according to RFC 7396
  • Remove omitempty from new .pb.go files

Checklist prior to review

  • Link to docs PR or issue --
  • Link to UI issue or PR --
  • Is the issue worked on linked? -- Deal with merging arrays in policies #5076
  • The PR does not hardcode values that might break projects that depend on kuma (e.g. "kumahq" as a image registry) --
  • The PR will work for both Linux and Windows, system specific functions like syscall.Mkfifo have equivalent implementation on the other OS --
  • Unit Tests --
  • E2E Tests --
  • Manual Universal Tests --
  • Manual Kubernetes Tests --
  • Do you need to update UPGRADE.md? -- No
  • Does it need to be backported according to the backporting policy? -- No
  • Do you need to explicitly set a > Changelog: entry here or add a ci/ label to run fewer/more tests?

Changelog: feat(policy): reimplemented matching for new policies

@lobkovilya
fix(*): KDS marshal to json for new policies
f373556 
Signed-off-by: Ilya Lobkov <ilya.lobkov@konghq.com>
@lobkovilya
fix(*): unit tests
354c01e 
Signed-off-by: Ilya Lobkov <ilya.lobkov@konghq.com>
@lobkovilya
fix(*): golden tests
89f38f4 
Signed-off-by: Ilya Lobkov <ilya.lobkov@konghq.com>
@lobkovilya
Merge branch 'master' into fix/new-policy-merge
3a8ae9c
@lobkovilya
fix(*): make check
962cd88 
Signed-off-by: Ilya Lobkov <ilya.lobkov@konghq.com>
@lobkovilya
fix(*): make check
82841ca 
Signed-off-by: Ilya Lobkov <ilya.lobkov@konghq.com>
@lobkovilya
fix(*): maek sed across ubuntu and darwin
a7a335d 
Signed-off-by: Ilya Lobkov <ilya.lobkov@konghq.com>
lahabana
lahabana reviewed Oct 3, 2022
View reviewed changes
lahabana
lahabana reviewed Oct 3, 2022
View reviewed changes
@lobkovilya
fix merge func, add unit test
a84d728 
Signed-off-by: Ilya Lobkov <ilya.lobkov@konghq.com>
@lobkovilya lobkovilya marked this pull request as ready for review October 3, 2022 13:50
@lobkovilya lobkovilya requested a review from a team as a code owner October 3, 2022 13:50
@lahabana lahabana linked an issue Oct 3, 2022 that may be closed by this pull request
Deal with merging arrays in policies #5076
Closed
@lobkovilya
fix(*): add reflect to create new 'conf'
2915de7 
Signed-off-by: Ilya Lobkov <ilya.lobkov@konghq.com>
@lobkovilya
fix(*): get rid of extra marshaling/unmarshaling
1edf203 
Signed-off-by: Ilya Lobkov <ilya.lobkov@konghq.com>
lahabana
lahabana reviewed Oct 5, 2022
View reviewed changes
@lahabana
Copy link
Contributor

lahabana commented Oct 5, 2022

Would it make sense to just add MarshallJSON methods to the existing proto objects?

@lobkovilya
Copy link
Contributor Author

Would it make sense to just add MarshallJSON methods to the existing proto objects?

Do you mean old policies? I'm not sure I understand what we want to solve with added MarshallJSON method

@lobkovilya
Merge branch 'master' into fix/new-policy-merge
638600a 
Signed-off-by: Ilya Lobkov <ilya.lobkov@konghq.com>

# Conflicts:
#	app/kumactl/cmd/install/testdata/install-control-plane.cni-enabled.golden.yaml
#	app/kumactl/cmd/install/testdata/install-control-plane.cni-experimental-enabled.golden.yaml
#	app/kumactl/cmd/install/testdata/install-control-plane.defaults.golden.yaml
#	app/kumactl/cmd/install/testdata/install-control-plane.gateway-api-present-not-enabled.yaml
#	app/kumactl/cmd/install/testdata/install-control-plane.gateway-api-present.yaml
#	app/kumactl/cmd/install/testdata/install-control-plane.global.golden.yaml
#	app/kumactl/cmd/install/testdata/install-control-plane.override-env-vars.golden.yaml
#	app/kumactl/cmd/install/testdata/install-control-plane.overrides.golden.yaml
#	app/kumactl/cmd/install/testdata/install-control-plane.registry.golden.yaml
#	app/kumactl/cmd/install/testdata/install-control-plane.tproxy-ebpf-experimental-enabled.golden.yaml
#	app/kumactl/cmd/install/testdata/install-control-plane.with-egress.golden.yaml
#	app/kumactl/cmd/install/testdata/install-control-plane.with-helm-set.yaml
#	app/kumactl/cmd/install/testdata/install-control-plane.with-helm-values.yaml
#	app/kumactl/cmd/install/testdata/install-control-plane.with-ingress.golden.yaml
#	app/kumactl/cmd/install/testdata/install-control-plane.zone.golden.yaml
#	app/kumactl/cmd/install/testdata/install-cp-helm/empty.golden.yaml
#	app/kumactl/cmd/install/testdata/install-cp-helm/fix4485.golden.yaml
#	app/kumactl/cmd/install/testdata/install-cp-helm/fix4496.golden.yaml
#	app/kumactl/cmd/install/testdata/install-cp-helm/fix4935.golden.yaml
#	app/kumactl/cmd/install/testdata/install-crds.all.golden.yaml
#	app/kumactl/cmd/install/testdata/install-crds.experimental-gatewayapi.golden.yaml
#	deployments/charts/kuma/crds/kuma.io_meshtraces.yaml
#	pkg/core/xds/testdata/rules/04.golden.yaml
#	pkg/core/xds/testdata/rules/04.policy.yaml
#	pkg/core/xds/testdata/rules/05.golden.yaml
#	pkg/core/xds/testdata/rules/05.policy.yaml
#	pkg/plugins/policies/meshtrace/api/v1alpha1/meshtrace.pb.go
#	pkg/plugins/policies/meshtrace/api/v1alpha1/schema.yaml
#	pkg/plugins/policies/meshtrace/k8s/crd/kuma.io_meshtraces.yaml
@lobkovilya
fix(*): golden files
a6f9fed 
Signed-off-by: Ilya Lobkov <ilya.lobkov@konghq.com>
@lobkovilya
fix(*): revert omitempty for non-lists, add '+nullable'
cb873b2 
Signed-off-by: Ilya Lobkov <ilya.lobkov@konghq.com>
@lobkovilya
make check
ab16e6a 
Signed-off-by: Ilya Lobkov <ilya.lobkov@konghq.com>
@lobkovilya lobkovilya force-pushed the fix/new-policy-merge branch from 25ba459 to ab16e6a Compare October 12, 2022 00:25
@lobkovilya
fix(*): update golden files
90597d8 
Signed-off-by: Ilya Lobkov <ilya.lobkov@konghq.com>
@lobkovilya
fix(*): add '+optional' marker
8a096e2 
Signed-off-by: Ilya Lobkov <ilya.lobkov@konghq.com>
@lobkovilya
make check
ca0930e 
Signed-off-by: Ilya Lobkov <ilya.lobkov@konghq.com>
@lobkovilya
fix(*): remove package optional marker
0a16de8 
Signed-off-by: Ilya Lobkov <ilya.lobkov@konghq.com>
@lobkovilya
fix(*): revert policy bootstrap change
613ea12 
Signed-off-by: Ilya Lobkov <ilya.lobkov@konghq.com>
@lobkovilya
Merge branch 'master' into fix/new-policy-merge
21db4be 
Signed-off-by: Ilya Lobkov <ilya.lobkov@konghq.com>

# Conflicts:
#	pkg/core/xds/testdata/rules/05.golden.yaml
#	pkg/core/xds/testdata/rules/05.policy.yaml
#	pkg/plugins/policies/matchers/testdata/match/dataplanepolicies/01.golden.yaml
#	pkg/plugins/policies/matchers/testdata/match/dataplanepolicies/02.golden.yaml
#	pkg/plugins/policies/meshtrace/api/v1alpha1/meshtrace.pb.go
@lobkovilya
make check
837d9eb 
Signed-off-by: Ilya Lobkov <ilya.lobkov@konghq.com>
@lobkovilya
fix(*): marshal in postgres store
5336a50 
Signed-off-by: Ilya Lobkov <ilya.lobkov@konghq.com>
@lobkovilya
fix(*): update kumactl
11190f6 
Signed-off-by: Ilya Lobkov <ilya.lobkov@konghq.com>
lahabana
lahabana reviewed Oct 17, 2022
View reviewed changes
Copy link
Contributor

@lahabana lahabana left a comment

Choose a reason for hiding this comment

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

It does make sense appart from the few comments. Though I'd love someone else more up to date with the code to double check this

@lobkovilya
fix(*): replace proto.Message with 'any'
f3fb251 
Signed-off-by: Ilya Lobkov <ilya.lobkov@konghq.com>
@lobkovilya
Merge branch 'master' into fix/new-policy-merge
aa472e4 
Signed-off-by: Ilya Lobkov <ilya.lobkov@konghq.com>

# Conflicts:
#	app/kumactl/cmd/install/testdata/install-control-plane.cni-enabled.golden.yaml
#	app/kumactl/cmd/install/testdata/install-control-plane.cni-experimental-enabled.golden.yaml
#	app/kumactl/cmd/install/testdata/install-control-plane.global.golden.yaml
#	app/kumactl/cmd/install/testdata/install-control-plane.override-env-vars.golden.yaml
#	app/kumactl/cmd/install/testdata/install-control-plane.overrides.golden.yaml
#	app/kumactl/cmd/install/testdata/install-control-plane.registry.golden.yaml
#	app/kumactl/cmd/install/testdata/install-control-plane.tproxy-ebpf-experimental-enabled.golden.yaml
#	app/kumactl/cmd/install/testdata/install-control-plane.with-egress.golden.yaml
#	app/kumactl/cmd/install/testdata/install-control-plane.with-helm-values.yaml
#	app/kumactl/cmd/install/testdata/install-control-plane.with-ingress.golden.yaml
#	app/kumactl/cmd/install/testdata/install-control-plane.zone.golden.yaml
#	app/kumactl/cmd/install/testdata/install-cp-helm/empty.golden.yaml
#	app/kumactl/cmd/install/testdata/install-cp-helm/fix4485.golden.yaml
#	app/kumactl/cmd/install/testdata/install-cp-helm/fix4496.golden.yaml
#	app/kumactl/cmd/install/testdata/install-cp-helm/fix4935.golden.yaml
@lobkovilya
review
03b75e5 
Signed-off-by: Ilya Lobkov <ilya.lobkov@konghq.com>
@lobkovilya
Merge branch 'master' into fix/new-policy-merge
0126763
@lobkovilya lobkovilya merged commit d9d1e45 into kumahq:master Oct 18, 2022
@lobkovilya lobkovilya deleted the fix/new-policy-merge branch October 18, 2022 17:36
@lahabana lahabana mentioned this pull request Dec 13, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lahabana lahabana lahabana approved these changes

@jakubdyszkiewicz jakubdyszkiewicz jakubdyszkiewicz approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Deal with merging arrays in policies
3 participants
@lobkovilya @lahabana @jakubdyszkiewicz