Add two more CNS endpoints to the CNS client #1541
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rbtr
reviewed
Aug 23, 2022
rbtr
requested changes
Aug 23, 2022
rbtr
previously approved these changes
Aug 25, 2022
timraymond
force-pushed
the
traymond/more-cns-endpoints
branch
3 times, most recently
from
d1355d7 to
0b89fbf
Compare
timraymond
force-pushed
the
traymond/more-cns-endpoints
branch
from
0b89fbf to
bca6a18
Compare
rbtr
previously approved these changes
Sep 7, 2022
timraymond
force-pushed
the
traymond/more-cns-endpoints
branch
from
bca6a18 to
f4fc43c
Compare
3 tasks
rbtr
force-pushed
the
traymond/more-cns-endpoints
branch
from
f4fc43c to
2ad8778
Compare
timraymond
force-pushed
the
traymond/more-cns-endpoints
branch
5 times, most recently
from
089037b to
55b2dcb
Compare
rbtr
approved these changes
Sep 12, 2022
This is the last part of CNS's API surface that is used by DNC, but not covered by a method from the CNS client.
The error handling here is simple enough that it doesn't really warrant wrapping it with a lambda.
DNC uses this API to detect GRE key capabilities. Since it's not supported by the client, it does this with direct HTTP requests currently. In order to ensure that there's only one way of accessing CNS, this implements the method so that the client can be used in DNC instead.
This was forgotten in a previous iteration.
The NMAgentSupportedAPIs method did not error when a non-2XX status code was encountered. This leads to surprising behavior on the part of the consumer.
This was a linter suggestion, and a good one. http.NoBody is more semantically meaningful than passing a nil.
This is in response to a linter complaint that dynamic errors were being used instead of a static error. Declaring an error type with var wouldn't carry along necessary debugging information (namely the HTTP status code), so it wasn't really appropriate. Rather than disable the check entirely with a linter directive, this defines a reusable error type so that HTTP errors can be communicated upward consistently.
These really should be a single set of paths, but there's this existing block of constants to define a "contract with DNC." Whether or not that's complete, the spirit of it is somewhat clear. However, it's not great to be duplicating constants all over the place. This is somewhat of a compromise by defining the newer constants in terms of the older ones.
This was a good suggestion from the linter.
timraymond
force-pushed
the
traymond/more-cns-endpoints
branch
from
55b2dcb to
04777c0
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
These are the final two endpoints that DNC uses without going through the CNS client. Adding these makes it possible to use the CNS client exclusively in DNC.
Notes: