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

Add support for wildcard hostname in VirutalServer #2939

Merged
merged 41 commits into from
Aug 29, 2022
Merged
Show file tree
Hide file tree
Changes from 32 commits
Commits
Show all changes
41 commits
Select commit Hold shift + click to select a range
12e74ff
Example test to display error event for invalid annotation name
Jul 25, 2022
6b738f8
Merge branch 'main' of github.com:nginxinc/kubernetes-ingress
Jul 25, 2022
fa2fd08
Remove validation test
Jul 25, 2022
e66e5da
Merge branch 'main' of github.com:nginxinc/kubernetes-ingress
Jul 27, 2022
554292e
Merge branch 'main' of github.com:nginxinc/kubernetes-ingress
Aug 2, 2022
8e0ae97
Merge branch 'main' of github.com:nginxinc/kubernetes-ingress
Aug 3, 2022
92882bf
Merge branch 'main' of github.com:nginxinc/kubernetes-ingress
Aug 5, 2022
1ada9c1
Merge branch 'main' of github.com:nginxinc/kubernetes-ingress
Aug 8, 2022
c137caa
Merge branch 'main' of github.com:nginxinc/kubernetes-ingress
Aug 12, 2022
99422d7
Add support for wildcard hostnames in VirtualServer
Aug 16, 2022
04f78dc
Update buildDNSEndpoint to use vs.ObjectMeta.Name instead of vs.Spec.…
Aug 18, 2022
fa37731
Remove unused function
Aug 18, 2022
caee7d4
Merge branch 'main' into feat/wildcardhostname
shaun-nx Aug 18, 2022
4f96e56
Revert example configuration
Aug 18, 2022
3dd09b0
Add a const for wildcardHost symbol
Aug 18, 2022
6c72b3c
Add check for host name collision with wildcards
Aug 22, 2022
bc405c5
Revert host collision update
Aug 22, 2022
0b8d855
Merge branch 'main' of github.com:nginxinc/kubernetes-ingress
Aug 22, 2022
d84683b
Merge branch 'main' into feat/wildcardhostname
Aug 22, 2022
36c3461
Revert example files
Aug 22, 2022
ef31cb3
Update python test for externaldns
Aug 23, 2022
d26c69f
Merge branch 'main' into feat/wildcardhostname
shaun-nx Aug 23, 2022
3b5abb8
Fix error vs.ObjectMeta.Host undefined
Aug 23, 2022
acf1821
Merge branch 'main' of github.com:nginxinc/kubernetes-ingress
Aug 23, 2022
5a92f83
Merge branch 'main' into feat/wildcardhostname
Aug 23, 2022
908782a
Remove space
Aug 23, 2022
a2cb143
Merge branch 'main' into feat/wildcardhostname
shaun-nx Aug 23, 2022
0844f32
Merge branch 'main' into feat/wildcardhostname
shaun-nx Aug 24, 2022
8cd30af
Update documentation
Aug 24, 2022
321856b
Merge branch 'feat/wildcardhostname' of github.com:nginxinc/kubernete…
Aug 24, 2022
b25d676
add tests for vs wildcard support
vepatel Aug 24, 2022
29a19b9
lint and retry for vs status
vepatel Aug 24, 2022
56aaca2
fix retries
vepatel Aug 24, 2022
9d06ca5
isort
vepatel Aug 24, 2022
c301b4e
Validate wildcard host
Aug 24, 2022
0b80f9b
Merge branch 'main' of github.com:nginxinc/kubernetes-ingress
Aug 24, 2022
9660ffe
Merge branch 'main' into feat/wildcardhostname
Aug 24, 2022
7905670
Merge branch 'main' into feat/wildcardhostname
shaun-nx Aug 25, 2022
3ae9c3b
Merge branch 'main' into feat/wildcardhostname
shaun-nx Aug 25, 2022
cecc195
Invert logic
Aug 25, 2022
a01e7e2
Merge branch 'main' into feat/wildcardhostname
shaun-nx Aug 29, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions deployments/service/loadbalancer-aws-elb.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ metadata:
name: nginx-ingress
namespace: nginx-ingress
annotations:
service.beta.kubernetes.io/aws-load-balancer-backend-protocol: "tcp"
service.beta.kubernetes.io/aws-load-balancer-proxy-protocol: "*"
service.beta.kubernetes.io/aws-load-balancer-type: "nlb"
service.beta.kubernetes.io/aws-load-balancer-nlb-target-type: "ip"
Copy link
Contributor

Choose a reason for hiding this comment

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

minor one but these 2 annotations can be appended to old list instead of replacing them

spec:
type: LoadBalancer
ports:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ spec:
{{% table %}}
|Field | Description | Type | Required |
| ---| ---| ---| --- |
|``host`` | The host (domain name) of the server. Must be a valid subdomain as defined in RFC 1123, such as ``my-app`` or ``hello.example.com``. Wildcard domains like ``*.example.com`` are not allowed. The ``host`` value needs to be unique among all Ingress and VirtualServer resources. See also [Handling Host and Listener Collisions](/nginx-ingress-controller/configuration/handling-host-and-listener-collisions). | ``string`` | Yes |
|``host`` | The host (domain name) of the server. Must be a valid subdomain as defined in RFC 1123, such as ``my-app`` or ``hello.example.com``. When using a wildcard domain like ``*.example.com`` the domain must be contained in double quotes. The ``host`` value needs to be unique among all Ingress and VirtualServer resources. See also [Handling Host and Listener Collisions](/nginx-ingress-controller/configuration/handling-host-and-listener-collisions). | ``string`` | Yes |
Copy link
Contributor

Choose a reason for hiding this comment

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

The ``host`` value needs to be unique: do we allow two VS resources with same wildcard host? eg. both with *.something.com

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. I'll check if that's possible or not.

Copy link
Contributor Author

@shaun-nx shaun-nx Aug 29, 2022

Choose a reason for hiding this comment

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

We don't allow two VS resources to be deployed with the same wildcard hostname. Output below is what I got when I tried this:

Spec:
  Host:  *.example.com
  Routes:
    Action:
      Pass:  tea
    Path:    /tea
    Action:
      Pass:  coffee
    Path:    /coffee
  Tls:
    Secret:  cafe-secret
  Upstreams:
    Name:     tea
    Port:     80
    Service:  tea-svc
    Name:     coffee
    Port:     80
    Service:  coffee-svc
Status:
  Message:  Host is taken by another resource
  Reason:   Rejected
  State:    Warning
Events:
  Type     Reason    Age   From                      Message
  ----     ------    ----  ----                      -------
  Warning  Rejected  13s   nginx-ingress-controller  Host is taken by another resource

|``tls`` | The TLS termination configuration. | [tls](#virtualservertls) | No |
|``externalDNS`` | The externalDNS configuration for a VirtualServer. | [externalDNS](#virtualserverexternaldns) | No |
|``dos`` | A reference to a DosProtectedResource, setting this enables DOS protection of the VirtualServer. | ``string`` | No |
Expand Down Expand Up @@ -249,7 +249,7 @@ Note that each subroute must have a `path` that starts with the same prefix (her
{{% table %}}
|Field | Description | Type | Required |
| ---| ---| ---| --- |
|``host`` | The host (domain name) of the server. Must be a valid subdomain as defined in RFC 1123, such as ``my-app`` or ``hello.example.com``. Wildcard domains like ``*.example.com`` are not allowed. Must be the same as the ``host`` of the VirtualServer that references this resource. | ``string`` | Yes |
|``host`` | The host (domain name) of the server. Must be a valid subdomain as defined in RFC 1123, such as ``my-app`` or ``hello.example.com``. When using a wildcard domain like ``*.example.com`` the domain must be contained in double quotes. Must be the same as the ``host`` of the VirtualServer that references this resource. | ``string`` | Yes |
|``upstreams`` | A list of upstreams. | [[]upstream](#upstream) | No |
|``subroutes`` | A list of subroutes. | [[]subroute](#virtualserverroutesubroute) | No |
|``ingressClassName`` | Specifies which Ingress Controller must handle the VirtualServerRoute resource. Must be the same as the ``ingressClassName`` of the VirtualServer that references this resource. | ``string``_ | No |
Expand Down
4 changes: 2 additions & 2 deletions internal/externaldns/sync.go
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ func buildDNSEndpoint(extdnsLister []extdnslisters.DNSEndpointLister, vs *vsapi.
var existingDNSEndpoint *extdnsapi.DNSEndpoint
var err error
for _, el := range extdnsLister {
existingDNSEndpoint, err = el.DNSEndpoints(vs.Namespace).Get(vs.Spec.Host)
existingDNSEndpoint, err = el.DNSEndpoints(vs.Namespace).Get(vs.ObjectMeta.Name)
if err == nil {
break
}
Expand All @@ -154,7 +154,7 @@ func buildDNSEndpoint(extdnsLister []extdnslisters.DNSEndpointLister, vs *vsapi.

dnsEndpoint := &extdnsapi.DNSEndpoint{
ObjectMeta: metav1.ObjectMeta{
Name: vs.Spec.Host,
Name: vs.ObjectMeta.Name,
Namespace: vs.Namespace,
Labels: vs.Labels,
OwnerReferences: []metav1.OwnerReference{*metav1.NewControllerRef(vs, controllerGVK)},
Expand Down
8 changes: 6 additions & 2 deletions pkg/apis/configuration/validation/virtualserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,15 +92,19 @@ func (vsv *VirtualServerValidator) validateVirtualServerSpec(spec *v1.VirtualSer
return allErrs
}

const wildcardHost = "*."

func validateHost(host string, fieldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{}

if host == "" {
return append(allErrs, field.Required(fieldPath, ""))
}

for _, msg := range validation.IsDNS1123Subdomain(host) {
allErrs = append(allErrs, field.Invalid(fieldPath, host, msg))
if !strings.HasPrefix(host, wildcardHost) {
Copy link
Contributor

Choose a reason for hiding this comment

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

We should also validate the wildcard host - we can use validation.IsWildcardDNS1123Subdomain

see here for reference

for _, msg := range validation.IsDNS1123Subdomain(host) {
allErrs = append(allErrs, field.Invalid(fieldPath, host, msg))
}
}

return allErrs
Expand Down
1 change: 1 addition & 0 deletions pkg/apis/configuration/validation/virtualserver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ func TestValidateHost(t *testing.T) {
"hello",
"example.com",
"hello-world-1",
"*.example.com",
}

for _, h := range validHosts {
Expand Down
20 changes: 20 additions & 0 deletions tests/data/virtual-server-wildcard/virtual-server-wildcard.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
apiVersion: k8s.nginx.org/v1
kind: VirtualServer
metadata:
name: virtual-server-wildcard
spec:
host: "*.example.com"
upstreams:
- name: backend2
service: backend2-svc
port: 80
- name: backend1
service: backend1-svc
port: 80
routes:
- path: "/backend1"
action:
pass: backend1
- path: "/backend2"
action:
pass: backend2
8 changes: 4 additions & 4 deletions tests/suite/test_virtual_server_externaldns.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
from suite.custom_resources_utils import is_dnsendpoint_present
from suite.resources_utils import get_events, wait_before_test
from suite.vs_vsr_resources_utils import patch_virtual_server_from_yaml
from suite.yaml_utils import get_first_host_from_yaml, get_namespace_from_yaml
from suite.yaml_utils import get_name_from_yaml, get_namespace_from_yaml

VS_YAML = f"{TEST_DATA}/virtual-server-external-dns/standard/virtual-server.yaml"

Expand All @@ -27,11 +27,11 @@ def test_responses_after_setup(
self, kube_apis, crd_ingress_controller_with_ed, create_externaldns, virtual_server_setup
):
print("\nStep 1: Verify DNSEndpoint exists")
dns_name = get_first_host_from_yaml(VS_YAML)
dns_ep_name = get_name_from_yaml(VS_YAML)
retry = 0
dep = is_dnsendpoint_present(kube_apis.custom_objects, dns_name, virtual_server_setup.namespace)
dep = is_dnsendpoint_present(kube_apis.custom_objects, dns_ep_name, virtual_server_setup.namespace)
while dep == False and retry <= 60:
dep = is_dnsendpoint_present(kube_apis.custom_objects, dns_name, virtual_server_setup.namespace)
dep = is_dnsendpoint_present(kube_apis.custom_objects, dns_ep_name, virtual_server_setup.namespace)
retry += 1
wait_before_test(1)
print(f"DNSEndpoint not created, retrying... #{retry}")
Expand Down
54 changes: 54 additions & 0 deletions tests/suite/test_virtual_server_wildcard.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
import pytest
from settings import TEST_DATA
from suite.custom_assertions import wait_and_assert_status_code
from suite.custom_resources_utils import read_custom_resource
from suite.resources_utils import wait_before_test
from suite.vs_vsr_resources_utils import create_virtual_server_from_yaml, delete_virtual_server


@pytest.mark.vs
@pytest.mark.parametrize(
"crd_ingress_controller, virtual_server_setup",
[
(
{"type": "complete", "extra_args": [f"-enable-custom-resources"]},
{"example": "virtual-server", "app_type": "simple"},
)
],
indirect=True,
)
class TestVirtualServerWildcard:
def test_vs_status(self, kube_apis, crd_ingress_controller, virtual_server_setup):

wait_and_assert_status_code(200, virtual_server_setup.backend_1_url, virtual_server_setup.vs_host)
wait_and_assert_status_code(200, virtual_server_setup.backend_2_url, virtual_server_setup.vs_host)
wait_and_assert_status_code(404, virtual_server_setup.backend_1_url, "test.example.com")
wait_and_assert_status_code(404, virtual_server_setup.backend_2_url, "test.example.com")

# create virtual server with wildcard hostname
manifest_vs_wc = f"{TEST_DATA}/virtual-server-wildcard/virtual-server-wildcard.yaml"
vs_wc_name = create_virtual_server_from_yaml(
kube_apis.custom_objects, manifest_vs_wc, virtual_server_setup.namespace
)
wait_before_test()
response = read_custom_resource(
kube_apis.custom_objects,
virtual_server_setup.namespace,
"virtualservers",
vs_wc_name,
)
while not response["status"]:
response = read_custom_resource(
kube_apis.custom_objects,
virtual_server_setup.namespace,
"virtualservers",
vs_wc_name,
)

assert response["status"]["reason"] == "AddedOrUpdated" and response["status"]["state"] == "Valid"
wait_and_assert_status_code(200, virtual_server_setup.backend_1_url, "test.example.com")
wait_and_assert_status_code(200, virtual_server_setup.backend_2_url, "test.example.com")
wait_and_assert_status_code(404, virtual_server_setup.backend_1_url, "test.xexample.com")
wait_and_assert_status_code(404, virtual_server_setup.backend_2_url, "test.xexample.com")

delete_virtual_server(kube_apis.custom_objects, vs_wc_name, virtual_server_setup.namespace)