Skip to content

Commit

Permalink
feat: add support for HTTP01 Challenges on VirtualServer resources (#…
Browse files Browse the repository at this point in the history
…2759)

* Treat challenge ingress as VSR

* Add unit tests and tidy up
  • Loading branch information
ciarams87 authored Jun 22, 2022
1 parent 816747c commit 56c756d
Show file tree
Hide file tree
Showing 5 changed files with 245 additions and 1 deletion.
55 changes: 55 additions & 0 deletions internal/k8s/configuration.go
Original file line number Diff line number Diff line change
Expand Up @@ -358,6 +358,7 @@ type Configuration struct {
internalRoutesEnabled bool
isTLSPassthroughEnabled bool
snippetsEnabled bool
isCertManagerEnabled bool

lock sync.RWMutex
}
Expand All @@ -374,6 +375,7 @@ func NewConfiguration(
transportServerValidator *validation.TransportServerValidator,
isTLSPassthroughEnabled bool,
snippetsEnabled bool,
isCertManagerEnabled bool,
) *Configuration {
return &Configuration{
hosts: make(map[string]Resource),
Expand All @@ -400,6 +402,7 @@ func NewConfiguration(
internalRoutesEnabled: internalRoutesEnabled,
isTLSPassthroughEnabled: isTLSPassthroughEnabled,
snippetsEnabled: snippetsEnabled,
isCertManagerEnabled: isCertManagerEnabled,
}
}

Expand Down Expand Up @@ -1279,6 +1282,7 @@ func squashResourceChanges(changes []ResourceChange) []ResourceChange {
func (c *Configuration) buildHostsAndResources() (newHosts map[string]Resource, newResources map[string]Resource) {
newHosts = make(map[string]Resource)
newResources = make(map[string]Resource)
var challengesVSR []*conf_v1.VirtualServerRoute

// Step 1 - Build hosts from Ingress resources

Expand All @@ -1291,6 +1295,14 @@ func (c *Configuration) buildHostsAndResources() (newHosts map[string]Resource,

var resource *IngressConfiguration

if val := c.isChallengeIngress(ing); val {
// if using cert-manager with Ingress, the challenge Ingress must be Minion
// and this code won't be reached. With VS, the challenge Ingress must not be Minion.
vsr := c.convertIngressToVSR(ing)
challengesVSR = append(challengesVSR, vsr)
continue
}

if isMaster(ing) {
minions, childWarnings := c.buildMinionConfigs(ing.Spec.Rules[0].Host)
resource = NewMasterIngressConfiguration(ing, minions, childWarnings)
Expand Down Expand Up @@ -1324,6 +1336,11 @@ func (c *Configuration) buildHostsAndResources() (newHosts map[string]Resource,
vs := c.virtualServers[key]

vsrs, warnings := c.buildVirtualServerRoutes(vs)
for _, vsr := range challengesVSR {
if vs.Spec.Host == vsr.Spec.Host {
vsrs = append(vsrs, vsr)
}
}
resource := NewVirtualServerConfiguration(vs, vsrs, warnings)

newResources[resource.GetKeyWithKind()] = resource
Expand Down Expand Up @@ -1377,6 +1394,44 @@ func (c *Configuration) buildHostsAndResources() (newHosts map[string]Resource,
return newHosts, newResources
}

func (c *Configuration) isChallengeIngress(ing *networking.Ingress) bool {
if !c.isCertManagerEnabled {
return false
}
return ing.Labels["acme.cert-manager.io/http01-solver"] == "true"
}

func (c *Configuration) convertIngressToVSR(ing *networking.Ingress) *conf_v1.VirtualServerRoute {
rule := ing.Spec.Rules[0]

vs := &conf_v1.VirtualServerRoute{
ObjectMeta: metav1.ObjectMeta{
Namespace: ing.Namespace,
Name: ing.Name,
},
Spec: conf_v1.VirtualServerRouteSpec{
Host: rule.Host,
Upstreams: []conf_v1.Upstream{
{
Name: "challenge",
Service: rule.HTTP.Paths[0].Backend.Service.Name,
Port: uint16(rule.HTTP.Paths[0].Backend.Service.Port.Number),
},
},
Subroutes: []conf_v1.Route{
{
Path: rule.HTTP.Paths[0].Path,
Action: &conf_v1.Action{
Pass: "challenge",
},
},
},
},
}

return vs
}

func (c *Configuration) buildMinionConfigs(masterHost string) ([]*MinionConfiguration, map[string][]string) {
var minionConfigs []*MinionConfiguration
childWarnings := make(map[string][]string)
Expand Down
154 changes: 154 additions & 0 deletions internal/k8s/configuration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ func createTestConfiguration() *Configuration {
validation.NewTransportServerValidator(isTLSPassthroughEnabled, snippetsEnabled, isPlus),
isTLSPassthroughEnabled,
snippetsEnabled,
certManagerEnabled,
)
}

Expand Down Expand Up @@ -2676,6 +2677,88 @@ func TestPortCollisions(t *testing.T) {
}
}

func TestChallengeIngressToVSR(t *testing.T) {
configuration := createTestConfiguration()

var expectedProblems []ConfigurationProblem

// Add a new Ingress

vs := createTestVirtualServer("virtualserver", "foo.example.com")
vsr1 := createTestChallengeVirtualServerRoute("challenge", "foo.example.com", "/.well-known/acme-challenge/test")

ing := createTestChallengeIngress("challenge", "foo.example.com", "/.well-known/acme-challenge/test", "cm-acme-http-solver-test")

expectedChanges := []ResourceChange{
{
Op: AddOrUpdate,
Resource: &VirtualServerConfiguration{
VirtualServer: vs,
VirtualServerRoutes: []*conf_v1.VirtualServerRoute{vsr1},
Warnings: nil,
},
},
}

configuration.AddOrUpdateVirtualServer(vs)
changes, problems := configuration.AddOrUpdateIngress(ing)
if diff := cmp.Diff(expectedChanges, changes); diff != "" {
t.Errorf("AddOrUpdateIngress() returned unexpected result (-want +got):\n%s", diff)
}
if diff := cmp.Diff(expectedProblems, problems); diff != "" {
t.Errorf("AddOrUpdateIngress() returned unexpected result (-want +got):\n%s", diff)
}

expectedChanges = nil

changes, problems = configuration.DeleteIngress(ing.Name)
if diff := cmp.Diff(expectedChanges, changes); diff != "" {
t.Errorf("DeleteIngress() returned unexpected result (-want +got):\n%s", diff)
}
if diff := cmp.Diff(expectedProblems, problems); diff != "" {
t.Errorf("DeleteIngress() returned unexpected result (-want +got):\n%s", diff)
}

expectedChanges = nil
ing = createTestIngress("wrong-challenge", "foo.example.com", "bar.example.com")
ing.Labels = map[string]string{"acme.cert-manager.io/http01-solver": "true"}
expectedProblems = []ConfigurationProblem{
{
Object: ing,
IsError: true,
Reason: "Rejected",
Message: "spec.rules: Forbidden: challenge Ingress must have exactly 1 rule defined",
},
}

changes, problems = configuration.AddOrUpdateIngress(ing)
if diff := cmp.Diff(expectedChanges, changes); diff != "" {
t.Errorf("AddOrUpdateIngress() returned unexpected result (-want +got):\n%s", diff)
}
if diff := cmp.Diff(expectedProblems, problems); diff != "" {
t.Errorf("AddOrUpdateIngress() returned unexpected result (-want +got):\n%s", diff)
}

ing = createTestIngress("wrong-challenge", "foo.example.com")
ing.Labels = map[string]string{"acme.cert-manager.io/http01-solver": "true"}
expectedProblems = []ConfigurationProblem{
{
Object: ing,
IsError: true,
Reason: "Rejected",
Message: "spec.rules.HTTP.Paths: Forbidden: challenge Ingress must have exactly 1 path defined",
},
}

changes, problems = configuration.AddOrUpdateIngress(ing)
if diff := cmp.Diff(expectedChanges, changes); diff != "" {
t.Errorf("AddOrUpdateIngress() returned unexpected result (-want +got):\n%s", diff)
}
if diff := cmp.Diff(expectedProblems, problems); diff != "" {
t.Errorf("AddOrUpdateIngress() returned unexpected result (-want +got):\n%s", diff)
}
}

func mustInitGlobalConfiguration(c *Configuration, gc *conf_v1alpha1.GlobalConfiguration) {
changes, problems, err := c.AddOrUpdateGlobalConfiguration(gc)

Expand Down Expand Up @@ -2740,6 +2823,50 @@ func createTestIngress(name string, hosts ...string) *networking.Ingress {
}
}

func createTestChallengeIngress(name string, host string, path string, serviceName string) *networking.Ingress {
var rules []networking.IngressRule
backend := networking.IngressBackend{
Service: &networking.IngressServiceBackend{
Name: serviceName,
Port: networking.ServiceBackendPort{
Number: 8089,
},
},
}

rules = append(rules, networking.IngressRule{
Host: host,
IngressRuleValue: networking.IngressRuleValue{
HTTP: &networking.HTTPIngressRuleValue{
Paths: []networking.HTTPIngressPath{
{
Path: path,
Backend: backend,
},
},
},
},
},
)

return &networking.Ingress{
ObjectMeta: metav1.ObjectMeta{
Name: name,
Namespace: "default",
CreationTimestamp: metav1.Now(),
Annotations: map[string]string{
"kubernetes.io/ingress.class": "nginx",
},
Labels: map[string]string{
"acme.cert-manager.io/http01-solver": "true",
},
},
Spec: networking.IngressSpec{
Rules: rules,
},
}
}

func createTestVirtualServer(name string, host string) *conf_v1.VirtualServer {
return &conf_v1.VirtualServer{
ObjectMeta: metav1.ObjectMeta{
Expand Down Expand Up @@ -2783,6 +2910,33 @@ func createTestVirtualServerRoute(name string, host string, path string) *conf_v
}
}

func createTestChallengeVirtualServerRoute(name string, host string, path string) *conf_v1.VirtualServerRoute {
return &conf_v1.VirtualServerRoute{
ObjectMeta: metav1.ObjectMeta{
Namespace: "default",
Name: name,
},
Spec: conf_v1.VirtualServerRouteSpec{
Host: host,
Upstreams: []conf_v1.Upstream{
{
Name: "challenge",
Service: "cm-acme-http-solver-test",
Port: 8089,
},
},
Subroutes: []conf_v1.Route{
{
Path: path,
Action: &conf_v1.Action{
Pass: "challenge",
},
},
},
},
}
}

func createTestTransportServer(name string, listenerName string, listenerProtocol string) *conf_v1alpha1.TransportServer {
return &conf_v1alpha1.TransportServer{
ObjectMeta: metav1.ObjectMeta{
Expand Down
4 changes: 3 additions & 1 deletion internal/k8s/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -342,7 +342,9 @@ func NewLoadBalancerController(input NewLoadBalancerControllerInput) *LoadBalanc
input.GlobalConfigurationValidator,
input.TransportServerValidator,
input.IsTLSPassthroughEnabled,
input.SnippetsEnabled)
input.SnippetsEnabled,
input.CertManagerEnabled,
)

lbc.appProtectConfiguration = appprotect.NewConfiguration()
lbc.dosConfiguration = appprotectdos.NewConfiguration(input.AppProtectDosEnabled)
Expand Down
4 changes: 4 additions & 0 deletions internal/k8s/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,10 @@ func isMaster(ing *networking.Ingress) bool {
return ing.Annotations["nginx.org/mergeable-ingress-type"] == "master"
}

func isChallengeIngress(ing *networking.Ingress) bool {
return ing.Labels["acme.cert-manager.io/http01-solver"] == "true"
}

// hasChanges determines if current ingress has changes compared to old ingress
func hasChanges(old *networking.Ingress, current *networking.Ingress) bool {
old.Status.LoadBalancer.Ingress = current.Status.LoadBalancer.Ingress
Expand Down
29 changes: 29 additions & 0 deletions internal/k8s/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -406,6 +406,35 @@ func validateIngress(
allErrs = append(allErrs, validateMinionSpec(&ing.Spec, field.NewPath("spec"))...)
}

if isChallengeIngress(ing) {
allErrs = append(allErrs, validateChallengeIngress(&ing.Spec, field.NewPath("spec"))...)
}

return allErrs
}

func validateChallengeIngress(spec *networking.IngressSpec, fieldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{}
if spec.Rules == nil || len(spec.Rules) != 1 {
allErrs = append(allErrs, field.Forbidden(fieldPath.Child("rules"), "challenge Ingress must have exactly 1 rule defined"))
return allErrs
}
r := spec.Rules[0]

if r.HTTP == nil || r.HTTP.Paths == nil || len(r.HTTP.Paths) != 1 {
allErrs = append(allErrs, field.Forbidden(fieldPath.Child("rules.HTTP.Paths"), "challenge Ingress must have exactly 1 path defined"))
return allErrs
}

p := r.HTTP.Paths[0]

if p.Backend.Service == nil {
allErrs = append(allErrs, field.Required(fieldPath.Child("rules.HTTP.Paths[0].Backend.Service"), "challenge Ingress must have a Backend Service defined"))
}

if p.Backend.Service.Port.Name != "" {
allErrs = append(allErrs, field.Forbidden(fieldPath.Child("rules.HTTP.Paths[0].Backend.Service.Port.Name"), "challenge Ingress must have a Backend Service Port Number defined, not Name"))
}
return allErrs
}

Expand Down

0 comments on commit 56c756d

Please sign in to comment.