add docker support #298
add docker support #298
Conversation
patrickdemers6
force-pushed
the
add-docker
branch
from
670c7af to
972820a
Compare
README.md
Outdated
| @@ -102,6 +122,13 @@ in lieu of command-line flags when using the above applications: | |||
| reduces both latency and the number of Fleet API calls a client makes when | |||
| reconnecting to a vehicle after restarting. This is particularly helpful | |||
| when using `tesla-control`, which restarts on each invocation. | |||
| * `TESLA_TLS_CERT` specifies a TLS certificate file for the HTTP proxy. | |||
There was a problem hiding this comment.
Does the Fleet Telemetry server also allow configuration through environment variables? This could become confusing.
There was a problem hiding this comment.
It's generally configured with config.json instead of flags/env vars.
| ) | ||
|
|
||
| func init() { | ||
| flag.StringVar(&httpConfig.certFilename, "cert", "", "TLS certificate chain `file` with concatenated server, intermediate CA, and root CA certificates") |
There was a problem hiding this comment.
Is the root CA certificate required? They usually aren't presented by the server.
There was a problem hiding this comment.
It should be included if the cert is signed by a CA. Running the HTTP proxy without a cert specified fails.
This PR isn't touching the help message but happy to tweak if this is not accurate.
func http.ListenAndServeTLS(addr string, certFile string, keyFile string, handler http.Handler) error
// ListenAndServeTLS acts identically to ListenAndServe,
// except that it expects HTTPS connections. Additionally, files containing a
// certificate and matching private key for the server must be provided.
// If the certificate is signed by a certificate authority, the certFile should
// be the concatenation of the server's certificate, any intermediates, and the CA's certificate.
README.md
Outdated
| Run the image from Docker hub: | ||
|
|
||
| ```bash | ||
| docker run -v ./config:/config teslamotors/vehicle-command:latest -tls-key /config/tls-key.pem -cert /config/tls-cert.pem -key-file /config/fleet-key.pem -host |
There was a problem hiding this comment.
Can you patch up the example OpenSSL command used to generate a self-signed TLS certificate so that the filenames are consistent with this invocation?
It might make more sense to limit to docker build here, and then give the run command after explaining key/certificate requirements in the later section.
There was a problem hiding this comment.
Keeping docker run, but switching it to just printing usage. Not diving into all the args since they haven't been explained yet.
.github/workflows/publish.yml
Outdated
| with: | ||
| bump_version_scheme: ${{ github.event.inputs.version_bump }} | ||
| use_github_release_notes: true | ||
| tag_prefix: v |
There was a problem hiding this comment.
i think this repo uses this static file for version bump https://github.com/teslamotors/vehicle-command/blob/main/pkg/account/version.txt#L1
not sure if this will work as intended
There was a problem hiding this comment.
Yeah. I'm open to suggestions on how to manage this better, but the idea is to have client User Agent versions correspond to SDK versions. The Makefile updates it but go build/go get are more common than make.
There was a problem hiding this comment.
Revised publish.yml so it automatically creates a GitHub release and pushes to Docker hub when the version.txt file changes.
Open to suggestions on how to test the workflow before merging.
There was a problem hiding this comment.
Just tested this in a private repository and everything is working well.
GitHub release is auto created and the image is pushed to Docker with latest tag and version tag. When something is merged and version tag doesn't change, nothing is triggered.
There was a problem hiding this comment.
We will need to make sure the Docker secrets are set and the workflow runner has write access to the repository.
patrickdemers6
force-pushed
the
add-docker
branch
2 times, most recently
from
886c0de to
7464d71
Compare
patrickdemers6
force-pushed
the
add-docker
branch
from
7464d71 to
5ee02e0
Compare
Description
Adding a
Dockerfileto make it easier to run the tools provided. This will be most useful for those wishing to run the HTTP proxy in a production environment.This introduces environment variable support for all flags available on the HTTP proxy. Environment variables are more graceful to use than command line flags when working with Docker.
I've done manual end to end testing, but would appreciate someone else setting this up themselves to help validate.
Fixes #225
Type of change
Please select all options that apply to this change:
Checklist:
Confirm you have completed the following steps: