Repository: avenga/couper
Pull request #634 (2022-11-24 12:24:24)

Fix tls block merging
Fixup for our mTLS feature.

---
<details>
    <summary>Reviewer checklist</summary>
    <ul>
        <li>Read PR description: a summary about the changes is required</li>
        <li>Changelog updated</li>
        <li>Documentation: docs/{Reference, Cli, ...}, Docker and cli help/usage</li>
        <li>Pulled branch, manually tested</li>
        <li>Verified requirements are met</li>
        <li>Reviewed the code</li>
        <li>Reviewed the related tests</li>
    </ul>
</details>



Patch:
diff --git a/config/configload/merge.go b/config/configload/merge.go
index 9f4f5ee4..311632b1 100644
--- a/config/configload/merge.go
+++ b/config/configload/merge.go
@@ -54,7 +54,8 @@ func mergeServers(bodies []*hclsyntax.Body, proxies map[string]*hclsyntax.Block)
 
 		tlsDefinition struct {
 			*hclsyntax.Block
-			blocks namedBlocks
+			typeBlocks      map[string][]*hclsyntax.Block
+			preservedBlocks namedBlocks
 		}
 
 		namedAPIs  map[string]*apiDefinition
@@ -308,8 +309,9 @@ func mergeServers(bodies []*hclsyntax.Body, proxies map[string]*hclsyntax.Block)
 				} else if block.Type == tls {
 					if results[serverKey].tls == nil {
 						results[serverKey].tls = &tlsDefinition{
-							Block:  block,
-							blocks: make(namedBlocks),
+							Block:           block,
+							preservedBlocks: namedBlocks{},
+							typeBlocks:      map[string][]*hclsyntax.Block{},
 						}
 					}
 
@@ -317,13 +319,24 @@ func mergeServers(bodies []*hclsyntax.Body, proxies map[string]*hclsyntax.Block)
 						results[serverKey].tls.Body.Attributes[name] = attr
 					}
 
+					thisTypeBlocks := map[string][]*hclsyntax.Block{}
 					for _, subBlock := range block.Body.Blocks {
-						blockKey := ""
+						blockKey := subBlock.Type
 						if len(subBlock.Labels) > 0 {
-							blockKey = subBlock.Labels[0]
+							blockKey += "_" + subBlock.Labels[0]
+							results[serverKey].tls.preservedBlocks[blockKey] = subBlock // override
+						} else {
+							thisTypeBlocks[subBlock.Type] = append(thisTypeBlocks[subBlock.Type], subBlock)
 						}
-						results[serverKey].tls.blocks[blockKey] = subBlock
 					}
+
+					for t, v := range thisTypeBlocks { // reset per file
+						if len(v) > 0 {
+							results[serverKey].tls.typeBlocks[t] = v
+						}
+
+					}
+
 				} else {
 					results[serverKey].blocks[block.Type] = block
 				}
@@ -425,8 +438,11 @@ func mergeServers(bodies []*hclsyntax.Body, proxies map[string]*hclsyntax.Block)
 
 		if serverBlock.tls != nil {
 			var tlsCertificateBlocks hclsyntax.Blocks
-			for _, blockName := range getSortedMapKeys(serverBlock.tls.blocks) {
-				tlsCertificateBlocks = append(tlsCertificateBlocks, serverBlock.tls.blocks[blockName])
+			for _, subType := range getSortedMapKeys(serverBlock.tls.typeBlocks) {
+				tlsCertificateBlocks = append(tlsCertificateBlocks, serverBlock.tls.typeBlocks[subType]...)
+			}
+			for _, k := range getSortedMapKeys(serverBlock.tls.preservedBlocks) {
+				tlsCertificateBlocks = append(tlsCertificateBlocks, serverBlock.tls.preservedBlocks[k])
 			}
 			serverBlocks = append(serverBlocks, &hclsyntax.Block{
 				Type: tls,


Test patch:
diff --git a/config/configload/merge_internal_test.go b/config/configload/merge_internal_test.go
index 749ae269..97346935 100644
--- a/config/configload/merge_internal_test.go
+++ b/config/configload/merge_internal_test.go
@@ -26,12 +26,24 @@ func Test_mergeServers_ServerTLS(t *testing.T) {
     server_certificate {
       attr1 = "val1"
     }
+    server_certificate {
+      attr1 = "val2"
+    }
+    client_certificate {
+      attr1 = "val3"
+    }
   }
 }`}, `server {
   tls {
+    client_certificate {
+      attr1 = "val3"
+    }
     server_certificate {
       attr1 = "val1"
     }
+    server_certificate {
+      attr1 = "val2"
+    }
   }
 }
 `, false},
@@ -40,17 +52,50 @@ func Test_mergeServers_ServerTLS(t *testing.T) {
     server_certificate {
       attr1 = "val1"
     }
+    server_certificate {
+      attr1 = "val2"
+    }
+    server_certificate "named" {
+      attr1 = "val3"
+    }
+    client_certificate {
+      attr1 = "val1"
+    }
+    client_certificate {
+      attr1 = "val2"
+    }
+    client_certificate "named" {
+      attr1 = "val3"
+    }
   }
 }`, `server {
   tls {
     server_certificate {
-      attr2 = "val2"
+      attr2 = "val4"
+    }
+    server_certificate "named" {
+      attr2 = "val5"
+    }
+    client_certificate {
+      attr2 = "val6"
+    }
+    client_certificate "named" {
+      attr2 = "val7"
     }
   }
 }`}, `server {
   tls {
+    client_certificate {
+      attr2 = "val6"
+    }
     server_certificate {
-      attr2 = "val2"
+      attr2 = "val4"
+    }
+    client_certificate "named" {
+      attr2 = "val7"
+    }
+    server_certificate "named" {
+      attr2 = "val5"
     }
   }
 }
@@ -60,15 +105,39 @@ func Test_mergeServers_ServerTLS(t *testing.T) {
     server_certificate "example1.com" {
       attr1 = "val1"
     }
+    client_certificate "a" {
+      attr1 = "val2"
+    }
+    client_certificate {
+      attr1 = "val3"
+    }
   }
 }`, `server {
   tls {
     server_certificate "example2.com" {
       attr2 = "val2"
     }
+    server_certificate {
+      attr2 = "val3"
+    }
+    client_certificate "b" {
+      attr2 = "val6"
+    }
   }
 }`}, `server {
   tls {
+    client_certificate {
+      attr1 = "val3"
+    }
+    server_certificate {
+      attr2 = "val3"
+    }
+    client_certificate "a" {
+      attr1 = "val2"
+    }
+    client_certificate "b" {
+      attr2 = "val6"
+    }
     server_certificate "example1.com" {
       attr1 = "val1"
     }
@@ -199,9 +268,7 @@ func writeBlocks(blocks hclsyntax.Blocks) string {
 	f := hclwrite.NewEmptyFile()
 	root := f.Body()
 
-	for _, block := range blocks {
-		appendBlock(root, block)
-	}
+	appendSorted(root, blocks)
 
 	b := &bytes.Buffer{}
 	_, _ = f.WriteTo(b)
@@ -224,9 +291,7 @@ func appendBlock(parent *hclwrite.Body, block *hclsyntax.Block) {
 	writeBlock := gohcl.EncodeAsBlock(block, block.Type)
 	writeBlock.SetLabels(block.Labels)
 
-	for _, subBlock := range block.Body.Blocks {
-		appendBlock(writeBlock.Body(), subBlock)
-	}
+	appendSorted(writeBlock.Body(), block.Body.Blocks)
 
 	appendAttrs(writeBlock.Body(), block.Body.Attributes)
 
@@ -239,3 +304,17 @@ func appendAttrs(parent *hclwrite.Body, attributes hclsyntax.Attributes) {
 		parent.SetAttributeValue(attr.Name, v)
 	}
 }
+
+func appendSorted(parent *hclwrite.Body, blocks hclsyntax.Blocks) {
+	named := namedBlocks{}
+	for _, block := range blocks {
+		if len(block.Labels) > 0 {
+			named[block.Type+"_"+block.Labels[0]] = block
+		} else {
+			appendBlock(parent, block)
+		}
+	}
+	for _, k := range getSortedMapKeys(named) {
+		appendBlock(parent, named[k])
+	}
+}
diff --git a/server/testdata/mtls/03_couper.hcl b/server/testdata/mtls/03_couper.hcl
index 2deae6ca..4aa19a3d 100644
--- a/server/testdata/mtls/03_couper.hcl
+++ b/server/testdata/mtls/03_couper.hcl
@@ -2,7 +2,7 @@ server {
   hosts = ["*:4443"]
 
   endpoint "/" {
-    response {}
+    response {} # OK
   }
 
   tls {