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

NET-5912/service-defaults protocol validation #21593

Merged
merged 2 commits into from
Aug 26, 2024

Conversation

JadhavPoonam
Copy link
Contributor

@JadhavPoonam JadhavPoonam commented Aug 9, 2024

Description

  • Add validation for the protocol field for service-defaults. With this change, the CLI and HTTP API only allows values listed in the documentation.

Testing & Reproduction steps

  • Tested locally in dev mode for both HTTP API (v1/config) and CLI config write

Links

Ticket: https://hashicorp.atlassian.net/browse/NET-5912

PR Checklist

  • updated test coverage
  • external facing docs updated
  • appropriate backport labels added
  • not a security concern

@JadhavPoonam JadhavPoonam force-pushed the NET-5912/config-entry-protocol-validation branch from 0751dc1 to 1c92192 Compare August 15, 2024 15:26
@JadhavPoonam JadhavPoonam added pr/no-changelog PR does not need a corresponding .changelog entry pr/no-backport labels Aug 15, 2024
@JadhavPoonam JadhavPoonam force-pushed the NET-5912/config-entry-protocol-validation branch from 1c92192 to 1b68af1 Compare August 15, 2024 16:18
@JadhavPoonam JadhavPoonam changed the title format NET-5912/service-defaults protocol validation Aug 15, 2024
@JadhavPoonam JadhavPoonam force-pushed the NET-5912/config-entry-protocol-validation branch 2 times, most recently from caf53d4 to dac29ef Compare August 21, 2024 13:57
@JadhavPoonam JadhavPoonam marked this pull request as ready for review August 21, 2024 14:08
@JadhavPoonam JadhavPoonam force-pushed the NET-5912/config-entry-protocol-validation branch from dac29ef to 239db97 Compare August 21, 2024 15:08
Copy link
Collaborator

@dhiaayachi dhiaayachi left a comment

Choose a reason for hiding this comment

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

Hey @JadhavPoonam
I left a question before I can approve this change. That said, I find it really funny that all our test cases test for the actual field being with an unsupported value udp 😆

agent/structs/config_entry.go Show resolved Hide resolved
@JadhavPoonam JadhavPoonam force-pushed the NET-5912/config-entry-protocol-validation branch from 239db97 to 48e3b08 Compare August 22, 2024 13:58
@JadhavPoonam JadhavPoonam force-pushed the NET-5912/config-entry-protocol-validation branch 3 times, most recently from 221ae7a to 76fe462 Compare August 23, 2024 15:01
@JadhavPoonam JadhavPoonam force-pushed the NET-5912/config-entry-protocol-validation branch from 76fe462 to 6d73e47 Compare August 23, 2024 15:24
Copy link
Collaborator

@dhiaayachi dhiaayachi left a comment

Choose a reason for hiding this comment

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

LGTM! 🚀

@JadhavPoonam JadhavPoonam merged commit cc2c8fb into main Aug 26, 2024
91 checks passed
@JadhavPoonam JadhavPoonam deleted the NET-5912/config-entry-protocol-validation branch August 26, 2024 15:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr/no-backport pr/no-changelog PR does not need a corresponding .changelog entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants