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

[VPN] psk documentation and validation updates in resource/vpnaas_site_connection_v2 #2011

Merged
merged 4 commits into from Dec 1, 2022
Merged

[VPN] psk documentation and validation updates in resource/vpnaas_site_connection_v2 #2011

merged 4 commits into from Dec 1, 2022

Conversation

jrod-97
Copy link
Contributor

@jrod-97 jrod-97 commented Nov 30, 2022 •
edited by anton-sidelnikov

Summary of the Pull Request

I've updated the description of vpnaas_site_connection_v2 to reflect the real world requirements that are present for the PSK after I ran into some issues trying "any string" that was not accepted.

PR Checklist

  • Refers to: #xxx
  • Tests added/passed.
  • Documentation updated.
  • Schema updated.
  • Release notes added.

vladimirvshivkov
vladimirvshivkov previously approved these changes Nov 30, 2022
View reviewed changes
Copy link
Contributor

@vladimirvshivkov vladimirvshivkov left a comment

Choose a reason for hiding this comment

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

I suppose it is a good idea to make this clear.

@jrod-97
Copy link
Contributor Author

jrod-97 commented Dec 1, 2022 •
edited

I've based my description on the popup in the web covering that appears when trying to paste a psk with a question mark in it. It says: Spaces and the following special characters are not allowed: <>&?*'"
Then you need to update the docs of both to clarify.

The error message that is delivered via the API could also be improved. I got: The PSK \"?\" is invalid. And I was left wondering why my nice PSK is transmitted as ? and suspected a API error at first.

@anton-sidelnikov
Copy link
Member

anton-sidelnikov commented Dec 1, 2022 •
edited

@jrod-97
Yes, thank you, I will forward this to backend team. Also little bit rephrased doc. Added validation.

@anton-sidelnikov anton-sidelnikov changed the title update vpnaas_site_connection_v2 psk description [VPN] psk documentation and validation updates in resource/vpnaas_site_connection_v2 Dec 1, 2022
@jrod-97
Copy link
Contributor Author

jrod-97 commented Dec 1, 2022

The regex to validate is a nice addition. Should improve UX even more.
Thanks for forwarding to the backend team!

@artem-lifshits artem-lifshits added the gate Merge PR label Dec 1, 2022
@artem-lifshits artem-lifshits added gate Merge PR and removed gate Merge PR labels Dec 1, 2022
@otc-zuul
Copy link

otc-zuul bot commented Dec 1, 2022

Build succeeded.
https://zuul.otc-service.com/t/eco/buildset/d9329bdbe6694e369f6aa6590336fb85

✔️ build-otc-releasenotes SUCCESS in 4m 18s
✔️ otc-golangci-lint SUCCESS in 2m 46s
✔️ golang-make-test SUCCESS in 3m 04s
✔️ golang-make-vet SUCCESS in 2m 35s
✔️ tflint SUCCESS in 1m 44s
✔️ goreleaser-build SUCCESS in 7m 55s

@otc-zuul otc-zuul bot merged commit 88149c4 into opentelekomcloud:devel Dec 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Aloento Aloento Aloento approved these changes

@anton-sidelnikov anton-sidelnikov anton-sidelnikov approved these changes

@artem-lifshits artem-lifshits artem-lifshits approved these changes

@lego963 lego963 Awaiting requested review from lego963

@vladimirvshivkov vladimirvshivkov Awaiting requested review from vladimirvshivkov

Assignees
No one assigned
Labels
gate Merge PR
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@jrod-97 @anton-sidelnikov @Aloento @vladimirvshivkov @artem-lifshits