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

[Bug]: Consul full Url is not considered due to UriBuilder and consul call fails #329

Closed
sreenath-guduru opened this issue May 22, 2024 · 8 comments
Labels
bug Something isn't working

Comments

@sreenath-guduru
Copy link

sreenath-guduru commented May 22, 2024

Describe the bug

When consul Url is like https://testconsul.com/details/sources, the consul keys load fails due to the way UrlBuilder is implemented.

Here ...

var builder = new UriBuilder(Client.Config.Address)
{
Path = url
};

If we pass on Client.Config.Address = https://testconsul.com/details/sources, based on UriBuilder working model, it takes host and considers only https://testconsul.com.

So, in order to consider this whole Url, it should be something like this. OR may be expose interface to provide custom UriBuilder by consumers?

UriBuilder builder = new UriBuilder
{
Scheme = uri.Scheme,
Host = uri.Host,
Path = uri.AbsolutePath + url (which is input in that function),
Query = uri.Query,
Port = uri.Port
};

This only works if consul uri is root domain like http://consultest.com:9999 or something like this.

Appreciate if this issue is considered for fix.

If there are any config options in this package to handle this situation, that also would be fine if shared.

Steps To Reproduce

  1. Set Client.Config.Address = https://testconsul.com/details/sources
  2. Try to build it for Keys generation.
  3. Due to wrong Url config internally, it fails generating keys.
  4. This only works if consul uri is root domain like http://consultest.com:9999 or something like this.

Expected behavior

Even when Consul Uri is with added path like https://testconsul.com/details/sources, it should work to pull keys

Environment

  • OS: Windows 10/11/Server latest
  • Consul Version: 1.6.10.8 & 1.7.14.3
  • consultdotnet Version 1.6.10.8 & 1.7.14.3

Logs

No response

Additional context

No response

@sreenath-guduru sreenath-guduru added the bug Something isn't working label May 22, 2024
@marcin-krystianc
Copy link
Contributor

Hi @sreenath-guduru,

Thanks for your report. To implement a test for it, we need to be able to run the test server under a path (not under a root domain). I'm not sure if that is possible with the vanilla Consul agent. I guess an external web server is necessary? Would you be able to find out how to prepare a test environment for your case?

@jasonestell
Copy link

jasonestell commented May 24, 2024

We setup an ingress to access consul from an app on an ec2 instance.

Ingress

apiVersion: networking.k8s.io/v1
kind: Ingress
metadata:
  name: consul
  namespace: consul
  annotations: 
    nginx.ingress.kubernetes.io/rewrite-target: /$1
spec:
  ingressClassName: nginx-alb-internal
  rules:
  - host: '*.hostname.com'
    http:
      paths:
      - path: /ci/consul-ingress/?(.*)
        pathType: Prefix
        backend:
          service:
            name: consul-server
            port:
              number: 8500

We are trying to hit the consul from the app on ec2 usign the path https://client.hostname.com/ci/consul-ingress

@sreenath-guduru
Copy link
Author

Hi @sreenath-guduru,

Thanks for your report. To implement a test for it, we need to be able to run the test server under a path (not under a root domain). I'm not sure if that is possible with the vanilla Consul agent. I guess an external web server is necessary? Would you be able to find out how to prepare a test environment for your case?

Hi @marcin-krystianc , please check this response and let us know.

@marcin-krystianc
Copy link
Contributor

I used Nginx to test this locally. I have a 'work in progress' branch (https://github.com/marcin-krystianc/consuldotnet/tree/dev-20240528-consul_paths), but since Consul.NET was not designed to work with a Consul URI under a subpath, it will take some time to finish it without breaking backward compatibility.

  • nginx config:
server {
		listen 80;
		server_name yourdomain.com;

		location /consul/ {
			proxy_pass http://localhost:8500/;
			proxy_set_header Host $host;
			proxy_set_header X-Real-IP $remote_addr;
			proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
			proxy_set_header X-Forwarded-Proto $scheme;

			# Additional headers to handle Consul's path rewrites
			proxy_redirect default;
			sub_filter_once off;
			sub_filter 'href="/' 'href="/consul/';
			sub_filter 'src="/' 'src="/consul/';
			sub_filter 'action="/' 'action="/consul/';
		}
	}

@sreenath-guduru
Copy link
Author

sreenath-guduru commented May 28, 2024

I used Nginx to test this locally. I have a 'work in progress' branch (marcin-krystianc/consuldotnet@dev-20240528-consul_paths), but since Consul.NET was not designed to work with a Consul URI under a subpath, it will take some time to finish it without breaking backward compatibility.

  • nginx config:
server {
		listen 80;
		server_name yourdomain.com;

		location /consul/ {
			proxy_pass http://localhost:8500/;
			proxy_set_header Host $host;
			proxy_set_header X-Real-IP $remote_addr;
			proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
			proxy_set_header X-Forwarded-Proto $scheme;

			# Additional headers to handle Consul's path rewrites
			proxy_redirect default;
			sub_filter_once off;
			sub_filter 'href="/' 'href="/consul/';
			sub_filter 'src="/' 'src="/consul/';
			sub_filter 'action="/' 'action="/consul/';
		}
	}

Great, Thank you for working on this and please keep us posted. And yes, I agree, it should work with root url (like http://localhost:8500/) and subpath too. If possible, can you please give us any ETA so that we can plan internally based on that.

@marcin-krystianc
Copy link
Contributor

marcin-krystianc commented May 28, 2024

Great, Thank you for working on this and please keep us posted. And yes, I agree, it should work with root url (like http://localhost:8500/) and subpath too. If possible, can you please give us any ETA so that we can plan internally based on that.

I'll do my best to fix in June (it may be sooner but I cannot promise that).

@marcin-krystianc
Copy link
Contributor

Thanks @sreenath-guduru for your report, it's available in version https://www.nuget.org/packages/Consul/1.7.14.4

@sreenath-guduru
Copy link
Author

Thanks @sreenath-guduru for your report, it's available in version nuget.org/packages/Consul/1.7.14.4

Thank you, @marcin-krystianc

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants