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

Plugins: Add option to disable TLS in the socks proxy #79246

Merged
merged 6 commits into from
Dec 14, 2023

Conversation

PoorlyDefinedBehaviour
Copy link
Member

@PoorlyDefinedBehaviour PoorlyDefinedBehaviour commented Dec 7, 2023

What is this feature?

Adds allow_insecure: bool to secure_socks_datasource_proxy config section. The socks proxy won't use TLS when allow_insecure is set to true.

Why do we need this feature?

From the issue description:

Sometimes there's a use case for testing grafana's SOCKs backend locally, and this would be significantly easier if we could disable the TLS requirement for the socks backend.

Who is this feature for?

Plugin developers

Which issue(s) does this PR fix?:

Part of https://github.com/grafana/hosted-grafana/issues/4935

Depends on grafana/grafana-plugin-sdk-go#833

TODO:

  • Update the plugin sdk dependency to the correct version before merging

go.mod Outdated
@@ -475,6 +474,8 @@ require (
go.opentelemetry.io/proto/otlp v1.0.0 // indirect
)

require github.com/grafana/grafana-plugin-sdk-go v0.196.1-0.20231205151856-c5479f051888
Copy link
Member Author

Choose a reason for hiding this comment

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

Using a specific commit from my PR in the sdk repo. Will update to the correct sdk version after the PR in the sdk repo is merged.

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated to the sdk version v0.197.0

@PoorlyDefinedBehaviour PoorlyDefinedBehaviour added the no-backport Skip backport of PR label Dec 7, 2023
@PoorlyDefinedBehaviour PoorlyDefinedBehaviour changed the title Plugins: add option to disable TLS in the socks proxy [v10.3.x] Plugins: add option to disable TLS in the socks proxy Dec 7, 2023
@PoorlyDefinedBehaviour PoorlyDefinedBehaviour force-pushed the secure-socks-proxy/add-insecure-option branch from 1bf4615 to 13d59e2 Compare December 7, 2023 20:05
Copy link
Member

@marefr marefr left a comment

Choose a reason for hiding this comment

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

Great LGTM

Sidenote: It's convenient to use a replace directive in go.mod when temporary working with dependency update. You can use go mod edit -replace=github.com/grafana/grafana-plugin-sdk-go=github.com/grafana/grafana-plugin-sdk-go@<branch name or commit>.

| `client_cert` | The file path of the client public key | /etc/client.crt |
| `server_name` | The domain name of the proxy, used for SNI | proxy.grafana.svc.cluster.local |
| `proxy_address` | The address of the proxy | localhost:9090 |
| `allow_insecure` | Disable TLS in the socks proxy | localhost:9090 |
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we need to change the example value in the new line to be a boolean.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed. It was a copy pasting mistake

@PoorlyDefinedBehaviour PoorlyDefinedBehaviour changed the title [v10.3.x] Plugins: add option to disable TLS in the socks proxy Plugins: add option to disable TLS in the socks proxy Dec 11, 2023
@PoorlyDefinedBehaviour PoorlyDefinedBehaviour changed the title Plugins: add option to disable TLS in the socks proxy Plugins: Add option to disable TLS in the socks proxy Dec 11, 2023
@PoorlyDefinedBehaviour PoorlyDefinedBehaviour merged commit 58678f5 into main Dec 14, 2023
14 checks passed
@PoorlyDefinedBehaviour PoorlyDefinedBehaviour deleted the secure-socks-proxy/add-insecure-option branch December 14, 2023 15:16
@summerwollin summerwollin modified the milestones: 10.3.x, 10.3.0 Jan 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants