search

eclipse / kuksa.val

kuksa.val
Apache License 2.0
89 stars 52 forks source link

Remove dummy VSS signals... #673

Closed SebastianSchildt closed 9 months ago

SebastianSchildt commented 9 months ago

... and adding some databroker version info to VSS tree instead. This might be useful, and avoids corner cases in databroker, where it has an empty/no tree

Currently added data

getTest Client> getMetaData Vehicle.KUKSA.Databroker
[
    {
        "path": "Vehicle.KUKSA.Databroker.GitVersion",
        "metadata": {
            "data_type": "STRING",
            "entry_type": "ATTRIBUTE",
            "description": "Databroker version as reported by GIT"
        }
    },
    {
        "path": "Vehicle.KUKSA.Databroker.CargoVersion",
        "metadata": {
            "data_type": "STRING",
            "entry_type": "ATTRIBUTE",
            "description": "Databroker version as reported by GIT"
        }
    },
    {
        "path": "Vehicle.KUKSA.Databroker.CommitSha",
        "metadata": {
            "data_type": "STRING",
            "entry_type": "ATTRIBUTE",
            "description": "Commit Sha of current version"
        }
    }
]

If we consider this databroker data useful, I'd prefer to have it under a "special" path such as Vehicle._kuksa but there is currently a bug (#674 ) in databroker that prevents accessing such paths in all conditions.

This will solve #668

argerus commented 9 months ago

I don't think corner cases are an issue really. But it could make sense to be able to access this information using the same methods as when accessing other signals.

It does seem weird to place them under Vehicle though, as they are not really "Vehicle signals".

Kuksa.Databroker.Version or Vendor.Kuksa.Databroker.Version might make more sense. And if they are namespaced with Kuksa, is there really a need to add a leading underscore in there?

And should these signals require specific authorization in order to access them?

argerus commented 9 months ago

If we consider this databroker data useful, I'd prefer to have it under a "special" path such as Vehicle._kuksa

See discussion in #674, but to address this specific point here:

Instead of creating a "special" path by introducing a leading underscore (which as I understand it, you think should be allowed for any externally defined signal as well), an alternative is to make this a "special" path by placing it under a completely separate root (as suggested above).

That would guarantee that it would not clash with any externally defined signal if all of those are assumed to have Vehicle as the root.

SebastianSchildt commented 9 months ago

I know moved the data to Kuksa.Databroker and confirmed also, there are no issues when loading a VSS model starting at Vehicle

I think currently there is no need for any "special" authorisation required, and since it is now neatly separated, with authorisation enabled, it works "just as usual", so if somebody wants it, he can add the prefix to the token scope.

argerus commented 9 months ago

Looks fine to me.

Not sure if we really want the "cargo version" and "git version" differentiation. It would make more sense to just have single version (Kuksa.Databroker.Version). But that can be something for a future PR to handle I guess.

Something with dash seems to have failed in CI run.

Totally unrelated to this PR, but the dash check has nothing to do with linting. Wouldn't it make more sense to put it together with the check-spdx-headers check as combined "license check".

erikbosch commented 9 months ago

I triggered a re-run of the failed check and then it succeeded. It seemed to be some temporary hickup, a 403 error when trying to download something

erikbosch commented 9 months ago 
erik@debian3:~/kuksa.val/kuksa-client$ kuksa-client

     ⢀⣤⣶⣾⣿⢸⣿⣿⣷⣶⣤⡀
    ⣴⣿⡿⠋⣿⣿   ⠈⠙⢿⣿⣦
   ⣾⣿⠋  ⣿⣿  ⣶⣿  ⠙⣿⣷
  ⣸⣿⠇   ⣿⣿⠠⣾⡿⠃   ⠸⣿⣇  ⣶ ⣠⡶⠂ ⣶  ⢰⡆ ⢰⡆⢀⣴⠖ ⢠⡶⠶⠶⡦   ⣰⣶⡀
  ⣿⣿    ⠿⢿⣷⣦⡀     ⣿⣿  ⣿⢾⣏   ⣿  ⢸⡇ ⢸⡷⣿⡁  ⠘⠷⠶⠶⣦  ⢠⡟⠘⣷
  ⢹⣿⡆   ⣿⣶⠈⢻⣿⡆   ⢰⣿⡏  ⠿ ⠙⠷⠄ ⠙⠷⠶⠟⠁ ⠸⠇⠈⠻⠦ ⠐⠷⠶⠶⠟ ⠠⠿⠁ ⠹⠧
   ⢿⣿⣄  ⣿⣿  ⠿⣿  ⣠⣿⡿
    ⠻⣿⣷⡄⣿⣿   ⢀⣠⣾⣿⠟    kuksa-client CLI
     ⠈⠛⠇⢿⣿⣿⣿⣿⡿⠿⠛⠁     0.4.0.post34+git.5f4e01db.dirty

Default tokens directory: /home/erik/kuksa.val/kuksa_certificates/jwt

Connecting to VSS server at 127.0.0.1 port 55555 using KUKSA GRPC protocol.
TLS will not be used.
INFO 2023-09-29 11:13:05,230 kuksa_client.grpc No Root CA present, it will not be posible to use a secure connection!
INFO 2023-09-29 11:13:05,230 kuksa_client.grpc.aio Establishing insecure channel
gRPC channel connected.
Test Client> getValue Kuksa.Databroker.GitVersion  
{
    "path": "Kuksa.Databroker.GitVersion",
    "value": {
        "value": "N/A",
        "timestamp": "2023-09-29T08:45:20.334727+00:00"
    }
}

Test Client> quit
gRPC channel disconnected.
erik@debian3:~/kuksa.val/kuksa-client$ git describe --tags
0.4.0-34-g5f4e01d
erik@debian3:~/kuksa.val/kuksa-client$
SebastianSchildt commented 9 months ago

Whether the information is available depends on "in which state" the working copy is, when building, currently this is the same you you get when doing --help . (I did not look deeper into what we are doing when buiding here, and waht vergen is doing to get the information as opposed to the py build system)

As we had a discussion before how to "determine"/report versions (tags? Cargo?) before, here I opted the report verbatim what was available at build time. Currently I would see it as "debugging/engineering info, whre likely for "released" version Cargo version number is all that is needed, whereas the has unambiguously identifies where you were no matter the build.

SebastianSchildt commented 9 months ago

Put the CI "weirdness" in a separate issue https://github.com/eclipse/kuksa.val/issues/677

Changing this/or using it in clients potentially can be in other PRs.

Merging this for now

erikbosch commented 9 months ago

Whether the information is available depends on "in which state" the working copy is, when building, currently this is the same you you get when doing --help . (I did not look deeper into what we are doing when buiding here, and waht vergen is doing to get the information as opposed to the py build system)

As we had a discussion before how to "determine"/report versions (tags? Cargo?) before, here I opted the report verbatim what was available at build time. Currently I would see it as "debugging/engineering info, whre likely for "released" version Cargo version number is all that is needed, whereas the has unambiguously identifies where you were no matter the build.

For info @SebastianSchildt , I tested also on the "official master build" and also there I get N/A:


erik@debian3:~/kuksa.val/kuksa-client$ docker run --rm -it --net=host  ghcr.io/eclipse/kuksa.val/databroker:master --help
Commit Date:      2023-09-29T09:59:05.000000000Z
  Commit SHA:       f716273b74f7a2612307d0a810f3e581f35c893c
  Commit Branch:    master

  Package version:  0.4.0
  Debug build:      false

Usage: databroker [OPTIONS]

Options:
      --address <IP>            Bind address [env: KUKSA_DATA_BROKER_ADDR=0.0.0.0] [default: 127.0.0.1]
      --port <PORT>             Bind port [env: KUKSA_DATA_BROKER_PORT=55555] [default: 55555]
      --vss <FILE>              Populate data broker with VSS metadata from (comma-separated) list of files [env: KUKSA_DATA_BROKER_METADATA_FILE=vss_release_4.0.json]
      --jwt-public-key <FILE>   Public key used to verify JWT access tokens
      --tls-cert <FILE>         TLS certificate file (.pem)
      --tls-private-key <FILE>  TLS private key file (.key)
      --disable-authorization   Disable authorization
      --insecure                Allow insecure connections
  -h, --help                    Print help
  -V, --version                 Print version

What client reports:

Test Client> getValue Kuksa.Databroker.GitVersion
{
    "path": "Kuksa.Databroker.GitVersion",
    "value": {
        "value": "N/A",
        "timestamp": "2023-09-29T12:40:06.867812+00:00"
    }
}

Test Client> getValue Kuksa.Databroker.CargoVersion
{
    "path": "Kuksa.Databroker.CargoVersion",
    "value": {
        "value": "0.4.0",
        "timestamp": "2023-09-29T12:40:06.867818+00:00"
    }
}

Test Client> getValue Kuksa.Databroker.CommitSha   
{
    "path": "Kuksa.Databroker.CommitSha",
    "value": {
        "value": "f716273b74f7a2612307d0a810f3e581f35c893c",
        "timestamp": "2023-09-29T12:40:06.867820+00:00"
    }
}

Test Client>

But maybe there are cases where Kuksa.Databroker.GitVersion actually return something

SebastianSchildt commented 9 months ago

But maybe there are cases where Kuksa.Databroker.GitVersion actually return something

Seems no in our current build

% docker run -it --rm   ghcr.io/eclipse/kuksa.val/databroker:0.4.0   -V                       
Kuksa Data Broker a1c9d5083033273b861771cd291ea3f4d7189df4

(I think the "-V" is conditional, it tries to get "GIT Version" when possible, but falls back to commit hash if not). So in general: Nothing really "critical" but probably something too look into during a rainy winter night)