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

Wildcard support for multi-tenant interceptor #281

Open
comron opened this issue Oct 1, 2021 · 5 comments
Open

Wildcard support for multi-tenant interceptor #281

comron opened this issue Oct 1, 2021 · 5 comments
Labels
good first issue Good for newcomers Hacktoberfest Great issues to tackle for Hacktoberfest 2021 stale-bot-ignore All issues that should not be automatically closed by our stale bot

Comments

@comron
Copy link

comron commented Oct 1, 2021

Allow for wildcards when specifying Host headers and routing information for interceptor and scaler.

Use-Case

Some services might have multiple hostnames, say for multiple customers in the form of "customername.service.example.com". So supplying a single Host header for routing/scaling wont suffice. We would need to support simple wildcards (*.service.exmaple.com") at the very least for this case. It might also be desirable to have the host parameter be a list rather than a single value.

Specification

I'm not intimately familiar with the internals of the project but my first take would be:

  1. When defining the host in an HTTPScaledObject allow for wildcards (and possibly multiple values)
  2. The queue used for keeping track of pending requests should match an incoming Host header with all the hosts and wildcards it knows about.
  3. Update the routing table to handle routing for wildcard HTTPScaledObject
@arschles arschles added this to the v0.3.0 milestone Oct 1, 2021
@arschles
Copy link
Collaborator

arschles commented Oct 1, 2021

@comron sounds like a great idea! We'll get this implemented and released in v0.3.0.

@arschles arschles added the Hacktoberfest Great issues to tackle for Hacktoberfest 2021 label Oct 11, 2021
@arschles arschles modified the milestones: v0.3.0, v0.2.1 Nov 16, 2021
@arschles arschles added the good first issue Good for newcomers label Nov 16, 2021
@andresrsanchez
Copy link

Hi there!
So i did an initial implementation of this issue with the core logic in Table, something like:

func (t *Table) AddTarget(
	host string,
	target Target) error {
	n := strings.Count(host, "*")
	if n > 0 && (!strings.HasPrefix(host, "*") || n > 1) {
		return fmt.Errorf("invalid wildcard for host %s", host)
	} else if n > 0 {
		host = "^" + strings.Replace(host, "*", "[a-zA-Z0-9-]+", 1) + "$"
	}

	t.l.Lock()
	defer t.l.Unlock()
	_, err := t.lookup(host)
	if err == ErrTargetNotFound {
		t.m[host] = target
		return nil
	}
	return fmt.Errorf(
		"host %s is already registered in the routing table",
		host,
	)
}

func (t *Table) lookup(host string) (string, error) {
	_, ok := t.m[host]
	if ok {
		return host, nil
	}
	for k := range t.m {
		if matched, _ := regexp.MatchString(k, host); matched {
			return k, nil
		}
	}
	return "", ErrTargetNotFound
}

What do you all think about this? Should i simplify the solution?
Thanks!!!

@arschles
Copy link
Collaborator

On an initial look, this code looks good, but I'm on holiday looking at it from my iPad, so I'll take a more in-depth look when I'm back. Thank for putting it together!

@arschles
Copy link
Collaborator

arschles commented Jan 5, 2022

@andresrsanchez I looked in more depth at the code you posted and have a few questions/comments:

  1. Do you think we could support a (much) smaller subset of regexp functionality? I think starting with support for just * would satisfy (almost) all of this specific feature
  2. We could probably do lookups more time-efficiently in the lookup function, particularly if we just support * for now. One option would be to store the table in a tree (possibly a trie)

For an initial implementation, we definitely need to answer (1), but we don't necessarily need to have the most efficient implementation on our first PR. Let me know what you think

@arschles arschles modified the milestones: v0.3.0, v0.5.0 Jan 27, 2022
@stale
Copy link

stale bot commented Mar 28, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale All issues that are marked as stale due to inactivity label Mar 28, 2022
@arschles arschles added the stale-bot-ignore All issues that should not be automatically closed by our stale bot label Mar 29, 2022
@stale stale bot removed the stale All issues that are marked as stale due to inactivity label Mar 29, 2022
@tomkerkhove tomkerkhove removed this from the v0.5.0 milestone May 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers Hacktoberfest Great issues to tackle for Hacktoberfest 2021 stale-bot-ignore All issues that should not be automatically closed by our stale bot
Projects
Status: To Do
Development

No branches or pull requests

4 participants