caddytls: Consolidate empty APs more smartly (#7567)

* caddytls: Consoldate empty APs more smartly (fix #7559)

* Revise consolidation logic

Author: mholt

caddyconfig/httpcaddyfile/tlsapp.go

@@ -698,14 +698,31 @@ func consolidateAutomationPolicies(aps []*caddytls.AutomationPolicy) []*caddytls
 	emptyAPCount := 0
 	origLenAPs := len(aps)
 	// compute the number of empty policies (disregarding subjects) - see #4128
+	// while we're at it,
 	emptyAP := new(caddytls.AutomationPolicy)
 	for i := 0; i < len(aps); i++ {
 		emptyAP.SubjectsRaw = aps[i].SubjectsRaw
+		emptyAP.ManagersRaw = nil
 		if reflect.DeepEqual(aps[i], emptyAP) {
+			// AP is empty
 			emptyAPCount++
-			if !automationPolicyHasAllPublicNames(aps[i]) {
-				// if this automation policy has internal names, we might as well remove it
-				// so auto-https can implicitly use the internal issuer
+
+			// see if this AP shadows something later
+			shadowIdx := automationPolicyShadows(i, aps)
+			emptyAP.SubjectsRaw = nil
+			if shadowIdx >= 0 {
+				emptyAP.SubjectsRaw = aps[shadowIdx].SubjectsRaw
+				// allow the later policy, which is likely for a wildcard, to have cert
+				// managers ("get_certificate"), since wildcards now cover specific
+				// subdomains by default, when configured (see discussion in #7559)
+				emptyAP.ManagersRaw = aps[shadowIdx].ManagersRaw
+			}
+
+			// if this is the last AP, we can delete it, since auto-https should
+			// pick it up; if it shadows something later that is also empty, we
+			// can similarly delete this; but if it shadows something that is NOT
+			// empty, we must not delete it since the shadowing has a purpose
+			if i == len(aps)-1 || (shadowIdx >= 0 && reflect.DeepEqual(aps[shadowIdx], emptyAP)) {
 				aps = slices.Delete(aps, i, i+1)
 				i--
 			}

caddytest/integration/caddyfile_adapt/tls_automation_policies_11.caddyfiletest

@@ -54,11 +54,6 @@ b.com {
 								"via": "http"
 							}
 						]
-					},
-					{
-						"subjects": [
-							"b.com"
-						]
 					}
 				]
 			}

caddytest/integration/caddyfile_adapt/tls_automation_policies_12.caddyfiletest

@@ -0,0 +1,96 @@
+# example from https://github.com/caddyserver/caddy/issues/7559
+*.test.local {
+	tls {
+		get_certificate http http://cert-server:9000/certs
+	}
+	respond "wildcard"
+}
+
+# certificate for this subdomain is covered by wildcard above
+subdomain.test.local {
+	respond "subdomain"
+}
+
+----------
+{
+	"apps": {
+		"http": {
+			"servers": {
+				"srv0": {
+					"listen": [
+						":443"
+					],
+					"routes": [
+						{
+							"match": [
+								{
+									"host": [
+										"subdomain.test.local"
+									]
+								}
+							],
+							"handle": [
+								{
+									"handler": "subroute",
+									"routes": [
+										{
+											"handle": [
+												{
+													"body": "subdomain",
+													"handler": "static_response"
+												}
+											]
+										}
+									]
+								}
+							],
+							"terminal": true
+						},
+						{
+							"match": [
+								{
+									"host": [
+										"*.test.local"
+									]
+								}
+							],
+							"handle": [
+								{
+									"handler": "subroute",
+									"routes": [
+										{
+											"handle": [
+												{
+													"body": "wildcard",
+													"handler": "static_response"
+												}
+											]
+										}
+									]
+								}
+							],
+							"terminal": true
+						}
+					]
+				}
+			}
+		},
+		"tls": {
+			"automation": {
+				"policies": [
+					{
+						"subjects": [
+							"*.test.local"
+						],
+						"get_certificate": [
+							{
+								"url": "http://cert-server:9000/certs",
+								"via": "http"
+							}
+						]
+					}
+				]
+			}
+		}
+	}
+}