openconfig / gnoi

gRPC Network Operations Interface (gNOI) defines a set of gRPC-based microservices for executing operational commands on network devices.
Apache License 2.0
156 stars 67 forks source link

Update bootconfig.proto new #168

Closed sachendras closed 4 months ago

sachendras commented 4 months ago

Update bootconfig.proto to include updating security artifacts aswell part of the SetBootConfigRequest message.

Replaced https://github.com/openconfig/gnoi/pull/163 with the current PR to include changes pushed after the original Pull.

coveralls commented 4 months ago

Pull Request Test Coverage Report for Build 8104819386

Details


Totals Coverage Status
Change from base Build 8070276888: 0.0%
Covered Lines: 166
Relevant Lines: 13241

💛 - Coveralls
sachendras commented 4 months ago

Moving comments from https://github.com/openconfig/gnoi/pull/163.

by @xw-g: "gNSI.x client has access to those gNSI namespaces. I don't think we should add those in this API (which is about Bootconfig namespace)"

Response: gNSI.x Client would allow for reseting only the gNSI artifacts. But gNSI artifacts are referenced by the bootConfig and hence results in commit failures. The change in this pull will provide a single gRPC-based method to reset the boot config including the gNSI artifacts. This also provides parity with the original Bootstrapconfig that is provided by BootZ.

LimeHat commented 4 months ago

gNSI.x Client would allow for reseting only the gNSI artifacts. But gNSI artifacts are referenced by the bootConfig and hence results in commit failures.

Could you please elaborate on this scenario? What steps will lead to a failure?

sachendras commented 4 months ago

/gcbrun

sachendras commented 4 months ago

gNSI.x Client would allow for reseting only the gNSI artifacts. But gNSI artifacts are referenced by the bootConfig and hence results in commit failures.

Could you please elaborate on this scenario? What steps will lead to a failure?

This is in context to functional test runs at our end and how DUTs (and their different namespaces) should be completely reseted prior to running subsequent functional tests. In my last comment, I was referring to the possible use of gNSI.x client for an empty rotate operation to delete certs post a certZ test and returning the DUT back to the pool. This action may lead to broken dependencies and subsequent commit failures. However, the proposal in this Pull will helps us to restore bootconfig plus security artifacts via the same API.

LimeHat commented 4 months ago

I mean, the existence of isolated namespaces implies that you could run into dependency issues.. is the answer to define an RPC that merges the two namespaces into a single commit?

Can this be solved on the controller side? Reset the bootconfig first, then reset the gNSI profiles.

Also, I'm not sure that an empty rotate is the way to erase certZ test state. IMO with the certZ tests, the sequence should look like this: 1) test creates a new certz profile 2) reconfigures grpc-servers to use the profile (via gnoi.bootconfig if namespaces are in use) 3) performs necessary validations 4) reconfigures the servers back to the previous or default profile 5) deletes the certz profile

sachendras commented 4 months ago

I mean, the existence of isolated namespaces implies that you could run into dependency issues.. is the answer to define an RPC that merges the two namespaces into a single commit?

Can this be solved on the controller side? Reset the bootconfig first, then reset the gNSI profiles.

Also, I'm not sure that an empty rotate is the way to erase certZ test state. IMO with the certZ tests, the sequence should look like this:

  1. test creates a new certz profile
  2. reconfigures grpc-servers to use the profile (via gnoi.bootconfig if namespaces are in use)
  3. performs necessary validations
  4. reconfigures the servers back to the previous or default profile
  5. deletes the certz profile

For our test workflow (current use case), goal is to have a single RPC that allows for resetting Boot and Security namespaces in a single commit.

LimeHat commented 4 months ago

Fair enough (although now the service name is slightly misleading, since it affects more than just a bootConfig).

Having said that, you will not be able to completely reset security namespace using this rpc. gnsi.Certz certificates field in bootstrapData, which you're reusing here, can contain only certificates since it uses UploadRequest message (same thing is applicable for bootz.proto, but there, as far as I understand, this data was supposed to be used only when attestz/enrollz is not available to rotate from iDevID to oiDevID). It does not provide a capability to manage (delete) multiple profiles or key/cert data from a specific profile.

sachendras commented 4 months ago

when attestz/enrollz is not available to rotate from iDevID to oiDevID). It does not provide a capability to manage (delete) multiple profiles or key/cert data from a specific profile.

Right, let's use this Pull as a placeholder for now until we have https://github.com/openconfig/attestz/pull/36 resolved. This problem w/ new ssl profile exists w/ the current BootZ process as well. Therefore, we will have to revisit this pull.