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.

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

fix(kuma-dp) validate the DP proxy type #2186

Merged
merged 1 commit into from
Jun 29, 2021
Merged

fix(kuma-dp) validate the DP proxy type #2186

merged 1 commit into from
Jun 29, 2021

Conversation

jpeach
Copy link
Contributor

@jpeach jpeach commented Jun 21, 2021

Summary

Validate that the kuma-dp proxy type nust be "ingress" or "dataplane".

Full changelog

  • Fix kuma-dp --proxy-type flag validation

Issues resolved

N/A

Documentation

N/A

Testing

  • Unit tests
  • E2E tests
  • Manual testing on Universal
  • Manual testing on Kubernetes

@jpeach jpeach marked this pull request as ready for review June 23, 2021 12:03
@jpeach jpeach requested a review from a team as a code owner June 23, 2021 12:03
@lobkovilya
Copy link
Contributor

Thanks for fixing this! Look good, but probably it makes sense to move this check to Config#Validate method: https://github.com/kumahq/kuma/blob/master/pkg/config/app/kuma-dp/config.go#L191. There is already a call proxyType.IsValid() but I didn't take into account that proxyType contains gateway as well, which today we don't want to see as an option.

@jpeach
Copy link
Contributor Author

jpeach commented Jun 23, 2021

Thanks for fixing this! Look good, but probably it makes sense to move this check to Config#Validate method: https://github.com/kumahq/kuma/blob/master/pkg/config/app/kuma-dp/config.go#L191. There is already a call proxyType.IsValid() but I didn't take into account that proxyType contains gateway as well, which today we don't want to see as an option.

Ah I see. I agree it makes sense to validate there as well. I'd like to also keep the validation in run.go since that ensure we load the right resource type.

@jpeach
Copy link
Contributor Author

jpeach commented Jun 23, 2021

@lobkovilya I updated the validation PTAL

Validate that the kuma-dp proxy type must be "ingress" or "dataplane".
Add dataplane configuration validation to ensure the proxy type is both
valid and supported.

Signed-off-by: James Peach <james.peach@konghq.com>
@codecov-commenter
Copy link

Codecov Report

Merging #2186 (0483abb) into master (46e5b11) will increase coverage by 0.31%.
The diff coverage is 75.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2186      +/-   ##
==========================================
+ Coverage   51.75%   52.06%   +0.31%     
==========================================
  Files         909      913       +4     
  Lines       40998    41040      +42     
==========================================
+ Hits        21218    21367     +149     
+ Misses      17744    17614     -130     
- Partials     2036     2059      +23     
Impacted Files Coverage Δ
app/kuma-dp/cmd/run.go 71.32% <55.55%> (+2.31%) ⬆️
pkg/config/app/kuma-dp/config.go 67.34% <100.00%> (+1.75%) ⬆️
pkg/core/resources/manager/manager.go 80.64% <0.00%> (-3.23%) ⬇️
pkg/core/resources/model/rest/resource.go 57.40% <0.00%> (-1.86%) ⬇️
...k8s/native/controllers/proxytemplate_controller.go 0.00% <0.00%> (ø)
api/internal/util/proto/proto.go 62.50% <0.00%> (ø)
...kg/plugins/resources/k8s/native/pkg/test/within.go 100.00% <0.00%> (ø)
api/internal/util/proto/types.go 50.00% <0.00%> (ø)
api/observability/v1/mads.pb.go 35.94% <0.00%> (+1.30%) ⬆️
pkg/xds/generator/direct_access_proxy_generator.go 81.57% <0.00%> (+1.31%) ⬆️
... and 18 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 46e5b11...0483abb. Read the comment docs.

@jpeach jpeach merged commit 8b13853 into kumahq:master Jun 29, 2021
@jpeach jpeach deleted the fix/validate-dp-type branch June 29, 2021 01:20
mergify bot pushed a commit that referenced this pull request Jul 21, 2021
Validate that the kuma-dp proxy type must be "ingress" or "dataplane".
Add dataplane configuration validation to ensure the proxy type is both
valid and supported.

Signed-off-by: James Peach <james.peach@konghq.com>
(cherry picked from commit 8b13853)
jpeach added a commit that referenced this pull request Jul 21, 2021
Validate that the kuma-dp proxy type must be "ingress" or "dataplane".
Add dataplane configuration validation to ensure the proxy type is both
valid and supported.

Signed-off-by: James Peach <james.peach@konghq.com>
(cherry picked from commit 8b13853)

Co-authored-by: James Peach <james.peach@konghq.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants