Skip to content
This repository has been archived by the owner on Sep 2, 2024. It is now read-only.

DBC Feeder uses TLS for connection to kuksa.val Server by default #133

Open
sophokles73 opened this issue Aug 31, 2023 · 9 comments
Open

Comments

@sophokles73
Copy link
Contributor

The README of the dbc2val feeder suggests that the feeder will not use TLS for connecting to kuksa.val Server/Databroker.

However, ServerClientWrapper actually seems to default to using TLS.

We should either adapt the documentation or the implementation accordingly.

@SebastianSchildt
Copy link
Contributor

@sophokles73
Copy link
Contributor Author

Not if I provide my own config file that doesn't contain the tls option, or am I mistaken?

@SebastianSchildt
Copy link
Contributor

Ah that may be true, not sure without checking code. @erikbosch might know the answer immediately. He should be back really soon.

I agree, with the original issue: It should of course be consistent between code and docs

@sophokles73
Copy link
Contributor Author

Let me check this for you ;-)

https://github.com/eclipse/kuksa.val.feeders/blob/3a72e0b7230927205507e5fd96c5162cc26af56a/dbc2val/dbcfeeder.py#L483

@SebastianSchildt
Copy link
Contributor

In my opinion it is good that "the default is TLS unless you set it to false in config or cmd line", wile the default configfile could still be "false", as realistically that is what most people will use.

It should just be reflected in the docs.

@sophokles73
Copy link
Contributor Author

@sophokles73
Copy link
Contributor Author

I have taken another look at the supported command line switches and their defaults.
It seems strange to me that not all of the config properties can be configured using command line arguments and some of the implementation relies on default values set in a default config file included in the code tree instead of assuming reasonable defaults in the implementation itself.

For example, it seems to be impossible to disable usage of TLS on the command line, neither by means of a command line switch nor by means of an environment variable. Is that intended and follows a particular pattern (which I am not able to see) or is this simply a case of not implemented yet?

@erikbosch
Copy link
Contributor

We have an issue at eclipse/kuksa.val#589 on the fact that we are not that consistent in config options and the fact that we maybe should not support all config settings as command line arguments. There has also now and then been concerns that we should not change existing (default) behavior as that may cause regressions for downstream projects, I guess that is one of the reasons why broker and server had different default behavior. I will look more in detail and create a PR to better align behavior when time permits

@erikbosch
Copy link
Contributor

We have migrated CAN Provider (dbcfeeder) to a new repo. Long term my intention is to close all dbc/can-related issues in this repository. If we see the issue as still interesting or relevant we could create a new issue in the new repo, possibly referencing the old issue in the repository.

Suggested way forward:

  • I will migrate selected issues early 2024
  • If this issue has not been migrated but you want it to remain open please comment.
  • Remaining issues no one seems to be interested in will be closed around 2024-03-31

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants