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

Add validation of received NextHopEntry (and associated entries) to ensure validity. #79

Open
robshakir opened this issue Oct 10, 2021 · 0 comments
Open

Add validation of received NextHopEntry (and associated entries) to ensure validity. #79

robshakir opened this issue Oct 10, 2021 · 0 comments

Comments

@robshakir
Copy link
Contributor

Current implementation will accept an empty next-hop for example, where some fields must be populated (e.g., solely index is not valid).
robshakir added a commit that referenced this issue Oct 10, 2021
@robshakir
Send non-empty NH, improve client error output.
498dac0 
  * (M) client/client.go
    - Add String() method for operation result's details such that there
      is additional information as to what the error corresponds to.
  * (M) compliance/compliance.go
    - Ensure that we do not send an empty next-hop when at least one
      field must be populated. See #79 which tracks adding a fix to the
      server side code.
robshakir added a commit that referenced this issue Oct 12, 2021
@robshakir
[1/4] Compliance test fixes. (#80)
6a8cd9d 
* Add ccli parameters, update expected error response codes.

  * (M) client/client.go
    - Avoid panic when details are not populated.
  * (M) cmd/ccli/ccli_test.go
    - Add command line flag for skipping tests based on requiring FIB
      FIB ACK to allow skipping these tests where this is unimplemented.
  * (M) compliance/compliance.go
    - Add support for a caller to specify the election ID to be used.

* Send non-empty NH, improve client error output.

  * (M) client/client.go
    - Add String() method for operation result's details such that there
      is additional information as to what the error corresponds to.
  * (M) compliance/compliance.go
    - Ensure that we do not send an empty next-hop when at least one
      field must be populated. See #79 which tracks adding a fix to the
      server side code.

* Add support for allowing unimplemented in a check.

  * (M) chk/chk.go
    - Add options for checking errors that can be used to check whether
      something is unimplemented OR a specific error.
  * (M) compliance/compliance.go
    - Adopt this check for failed precondition vs. unimplemented for
    ALL_PRIMARY.

* [2/4] Bug fixes in test code. (#81)

* Bug fixes in test code.

  * (M) chk/chk.go
    - Add better error message formatting for debugging.
  * (M) client/client.go
    - Add a special case for a match used in compliance tests where the
      operation ID expected is unspecified.
  * (M) cmd/ccli/ccli_test.go
    - Rename some flags for consistency, allow tests that require
      reordering of transactions by the server to be skipped.
  * (M) compliance/compliance.go
    - Restructure the compliance tests to have a generator function that
      can be used to specify the ACK type, rather than needing to write
      wrappers manually.

* Specify InstalledInRIB as an argument.

  * (M) demo/ipv4/ipv4_test.go
  * (M) device/device_test.go
    - Adopt renamed tests.

* [3/4] Add compliance tests for metadata, different NI NHG. (#68)

* Add compliance tests for IPv4 entries, fix server defect.

  * (M) client/client.go
    - Improve logging.
  * (M) compliance/compliance.go
    - Add test for adding IPv4 entry in a single ModifyRequest,
     and within separate requests.
  * (M) fluent/fluent.go
    - Add support for WithIPAddress to builder.
  * (M) gnmit/gnmit.go
    - Minor refactor to use a new GNMIServer container - no-op
    change for adding gNMI Set support.
  * (M) rib/rib.go
    - Improve logging.
  * (M) server/server.go
  * (M) server/server_test.go
    - Resolve a defect whereby the server called modifyEntry within
      a goroutine per operation within a ModifyRequest. In the case
      where a single ModifyRequest made multiple entries resolvable,
      then an entry could be duplicate ACK'd, by all the entries that
      made it resolvable. Whilst the RIB is safe to modify in this
      manner, the pending queue could result in this occurring - the
      duplicate install is a NOOP, but the duplicate ACK violated the
      API contract. Initially, resolve this issue by serially processing
      modify requests - added a TODO to cover a less naive solution.

* Whitespace fixes.

* Add compliance tests for deleting entries.

  * (M) client/client.go
    - Improve debugging output of an operation result.
  * (M) compliance/compliance.go
    - Add tests for deleting IPv4 entries, NHGs, NHs - both positive
      and negative tests.
  * (M) fluent/fluent.go
    - Add ability to reference a NHG NI from an IPv4Entry.
  * (M) go.sum
    - Tidy up go modules.

* Fix tests to adopt new client stub argument.

* Add compliance tests for metadata, different NI NHG.

 * (M) client/client.go
  - Improve string message for OpResults.
 * (M) compliance/compliance.go
  - Add IPv4Entry -> NHG in different NI test - currently skipped
    based on server configuration.
  - Add test for metadata on IPv4Entry.
 * (M) fluent/fluent.go
  - Add fluent function for adding metadata.

* Fix support for comparing to unimplemented.

  * (M) chk/chk.go
    - Ensure that details are maintained when checking for
    unimplemented.
  * (M) compliance/compliance.go
    - Clean up compliance test.
  * (M) go.mod
  * (M) go.sum
    - Add golang gRPC genproto dependency.

* Update test names.

  * (M) demo/ipv4/ipv4_test.go
  * (M) device/device_test.go
    - Specify RIB ACK as an argument rather than using the removed test.

* [4/4] Handle nil responses when debugging. (#73)

* Do not send election_id = 0.

  * (M) compliance/compliance.go
    - Ensure that the compliance tests do not send election_id = 0
      which is an invalid value.

* Add server handling for election ID = 0.

  * (M) server/server.go
  * (M) server/server_test.go
    - Ensure that election ID set to zero is responded to with an
      error.

* Handle nil responses when debugging.

  * (M) client/client.go
  * (M) client/client_test.go
    - Ensure that a nil input value is cleanly handled when calling
      String() on a nil OpResult.

* Fix string error reporting for partially specified messages.

  * (M) client/client.go
  * (M) client/client_test.go
    - Fix a panic in OpResult.String that could occur for
      partially populated messages.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@robshakir