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.

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

Plumb additional TLS configuration #23

Merged
merged 2 commits into from
Apr 29, 2024
Merged

Plumb additional TLS configuration #23

merged 2 commits into from
Apr 29, 2024

Conversation

zalegrala
Copy link
Contributor

No description provided.

@zalegrala
Copy link
Contributor Author

zalegrala commented Mar 15, 2024

Testing this out doesn't seem to work correct. I have the following, but I don't think the new TLS values are being used. Any pointers?

const client = new tracing.Client({
  endpoint: 'https://ingest.trace.svc.cluster.local:4317',
  exporter: tracing.EXPORTER_OTLP,
  insecure: false,
  tls_server_name: 'tempo.trace.svc.cluster.local',
  tls_ca_file: '/tls/ca.crt',
  tls_cert_file: '/tls/tls.crt',
  tls_key_file: '/tls/tls.key',
  headers: { 'X-Scope-OrgID': randomOrg },
})

tracing.go Outdated
Comment on lines 149 to 153
Insecure bool `json:"insecure"`
TLSServerName string `json:"tls_server_name"`
TLSCertFile string `json:"tls_cert_file"`
TLSKeyFile string `json:"tls_key_file"`
TLSCAFile string `json:"tls_ca_file"`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WDYT of directly vendoring the otel TLS config block?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yes, good idea.

@zalegrala
Copy link
Contributor Author

I was able to validate these changes. The json tags on the config object are not honored, and instead, the default translation is required. This means that instead of the json tag of server_name_override on the go struct, the name of the struct is matched, and so just server_name is used. Additionally, to specify the CAFile where the struct has ca_file, users need to specify c_a_file.

@zalegrala zalegrala merged commit 721a4f4 into main Apr 29, 2024
2 checks passed
@zalegrala zalegrala deleted the zalegrala/tls branch April 29, 2024 21:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants