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

Handle missing TCP upstreams as warnings (#8461) #8469

Merged

Conversation

ashishb-solo
Copy link
Contributor

@ashishb-solo ashishb-solo commented Jul 14, 2023

  • Handle missing TCP upstreams as warnings

  • Move changelog to new location

  • Fix a broken test

  • Apply suggestions from code review

  • Fix error in changelog

  • Address nil safety issue on TcpHostError

  • Fix nil safety check

  • Address a few review comments

  • Update projects/gloo/api/grpc/validation/gloo_validation.proto

  • Populate ErrorLevel for TcpHostWarning

  • Mark InvalidDestinationError as deprecated

  • Add some stricter error checking

  • Move duplicated code into a reusable function

  • Codegen

  • Fix an error from refactor

  • Use the ErrorWithKnownLevel interface to switch on error report types

  • Handle the case where tcp host num is not provided

  • Provide a separate Context object to carry additional information

Earlier commits simply added more fields to the ErrorWithKnownLevel interface to aid in providing the necessary information required to report the error. This commit is an attempt to break that out into a separate object so additional fields do not need to be added every time more contextual information is needed to correctly report the error

  • Add docstrings on exported types

  • Adding changelog file to new location

  • Deleting changelog file from old location

  • Adding changelog file to new location

  • Deleting changelog file from old location

  • Clean up hard-coded warning type

Previously, we were just using InvalidDestinationWarning for all warning types

  • we remove that now and use a switch statement to use the right kind
  • Fix an inaccurate comment

Description

Please include a summary of the changes.

This bug fixes ... \ This new feature can be used to ...

Context

Users ran into this bug doing ... \ Users needed this feature to ...

Checklist:

  • I included a concise, user-facing changelog (for details, see https://github.com/solo-io/go-utils/tree/main/changelogutils) which references the issue that is resolved.
  • If I updated APIs (our protos) or helm values, I ran make -B install-go-tools generated-code to ensure there will be no code diff
  • I followed guidelines laid out in the Gloo Edge contribution guide
  • I opened a draft PR or added the work in progress label if my PR is not ready for review
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works

BOT NOTES:
resolves https://github.com/solo-io/solo-projects/issues/5163

* Handle missing TCP upstreams as warnings

* Move changelog to new location

* Fix a broken test

* Apply suggestions from code review

Co-authored-by: Nathan Fudenberg <[email protected]>

* Fix error in changelog

* Address nil safety issue on TcpHostError

* Fix nil safety check

* Address a few review comments

* Update projects/gloo/api/grpc/validation/gloo_validation.proto

Co-authored-by: Jenny Shu <[email protected]>

* Populate ErrorLevel for TcpHostWarning

* Mark InvalidDestinationError as deprecated

* Add some stricter error checking

* Move duplicated code into a reusable function

* Codegen

* Fix an error from refactor

* Use the ErrorWithKnownLevel interface to switch on error report types

* Handle the case where tcp host num is not provided

* Provide a separate Context object to carry additional information

Earlier commits simply added more fields to the ErrorWithKnownLevel interface
to aid in providing the necessary information required to report the error.
This commit is an attempt to break that out into a separate object so
additional fields do not need to be added every time more contextual
information is needed to correctly report the error

* Add docstrings on exported types

* Adding changelog file to new location

* Deleting changelog file from old location

* Adding changelog file to new location

* Deleting changelog file from old location

* Clean up hard-coded warning type

Previously, we were just using InvalidDestinationWarning for all warning types
- we remove that now and use a switch statement to use the right kind

* Fix an inaccurate comment

---------

Co-authored-by: soloio-bulldozer[bot] <48420018+soloio-bulldozer[bot]@users.noreply.github.com>
Co-authored-by: Nathan Fudenberg <[email protected]>
Co-authored-by: Jenny Shu <[email protected]>
Co-authored-by: changelog-bot <changelog-bot>
@ashishb-solo ashishb-solo requested a review from a team as a code owner July 14, 2023 15:36
@solo-changelog-bot
Copy link

Issues linked to changelog:
https://github.com/solo-io/solo-projects/issues/5163

@github-actions github-actions bot added the keep pr updated signals bulldozer to keep pr up to date with base branch label Jul 14, 2023
jenshu
jenshu previously approved these changes Jul 14, 2023
@soloio-bulldozer soloio-bulldozer bot merged commit 3870e91 into v1.14.x Jul 14, 2023
12 checks passed
@soloio-bulldozer soloio-bulldozer bot deleted the 5163-missing-tcp-upstreams-as-warnings-v1.14.x branch July 14, 2023 16:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
keep pr updated signals bulldozer to keep pr up to date with base branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants