Node Definition: Move Certificate SAN under Certificate stanza (#1821)

* Node Definition: Move Certificate SAN under Certificate stanza * fix * cleanup * remove unused param * remove from a caller * moved node sans under certificate --------- Co-authored-by: Roman Dodin <[email protected]>
srl-labs · Jan 17, 2024 · 475cd42 · 475cd42
1 parent be855fa
commit 475cd42
Show file tree

Hide file tree

Showing 10 changed files with 66 additions and 68 deletions.
diff --git a/clab/config.go b/clab/config.go
@@ -197,7 +197,6 @@ func (c *CLab) createNodeCfg(nodeName string, nodeDef *types.NodeDefinition, idx
 		Memory:          c.Config.Topology.GetNodeMemory(nodeName),
 		StartupDelay:    c.Config.Topology.GetNodeStartupDelay(nodeName),
 		AutoRemove:      c.Config.Topology.GetNodeAutoRemove(nodeName),
-		SANs:            c.Config.Topology.GetSANs(nodeName),
 		Extras:          c.Config.Topology.GetNodeExtras(nodeName),
 		WaitFor:         c.Config.Topology.GetWaitFor(nodeName),
 		DNS:             c.Config.Topology.GetNodeDns(nodeName),

diff --git a/docs/manual/nodes.md b/docs/manual/nodes.md
@@ -81,33 +81,6 @@ topology:
       image-pull-policy: Always
 ```
 
-### subject alternative names (SAN)
-
-With `SANs` the user sets the Subject Alternative Names that will be added to the node's certificate. Host names that are set by default are:
-
-For a topology node named "srl" in a lab named "srl01", the following SANs are set by default:
-
-- `srl`
-- `clab-srl01-srl`
-- `srl.srl01.io`
-- IPv4/6 addresses of the node
-
-```yaml
-name: srl01
-
-topology:
-  kinds:
-    nokia_srlinux:
-      type: ixrd3
-      image: ghcr.io/nokia/srlinux
-
-  nodes:
-    srl:
-      kind: nokia_srlinux
-      SANs:
-        - "test.com"
-```
-
 ### license
 
 Some containerized NOSes require a license to operate or can leverage a license to lift-off limitations of an unlicensed version. With `license` property a user sets a path to a license file that a node will use. The license file will then be mounted to the container by the path that is defined by the `kind/type` of the node.
@@ -751,6 +724,28 @@ To configure key size and certificate validity duration use the following option
     validity-duration: 1h
 ```
 
+#### subject alternative names (SAN)
+
+With `SANs` field of the certificate block the user sets the Subject Alternative Names that will be added to the node's certificate.
+
+For a topology node named "srl" in a lab named "srl01", the following SANs are set by default:
+
+- `srl`
+- `clab-srl01-srl`
+- `srl.srl01.io`
+- IPv4/6 addresses of the node
+
+```yaml
+topology:
+
+  nodes:
+    srl:
+      kind: nokia_srlinux
+      certificate:
+        SANs:
+          - "test.com"
+```
+
 ### healthcheck
 
 Containerlab supports the [docker healthcheck](https://docs.docker.com/engine/reference/builder/#healthcheck) configuration for the nodes. The healthcheck instruction can be set on the `defaults`, `kind` or `node` level, with the node level likely being the most used one.

diff --git a/nodes/default_node.go b/nodes/default_node.go
@@ -402,7 +402,15 @@ func (d *DefaultNode) LoadOrGenerateCertificate(certInfra *cert.Cert, topoName s
 			nodeConfig.LongName,
 			nodeConfig.ShortName + "." + topoName + ".io",
 		}
-		hosts = append(hosts, nodeConfig.SANs...)
+		// add the SANs provided via config
+		hosts = append(hosts, nodeConfig.Certificate.SANs...)
+
+		// add mgmt IPs as SANs to CSR
+		for _, ip := range []string{d.Cfg.MgmtIPv4Address, d.Cfg.MgmtIPv6Address} {
+			if ip != "" {
+				hosts = append(hosts, ip)
+			}
+		}
 
 		certInput := &cert.NodeCSRInput{
 			CommonName:   nodeConfig.ShortName + "." + topoName + ".io",

diff --git a/nodes/srl/srl.go b/nodes/srl/srl.go
@@ -244,13 +244,6 @@ func (s *srl) PreDeploy(_ context.Context, params *nodes.PreDeployParams) error
 func (s *srl) PostDeploy(ctx context.Context, params *nodes.PostDeployParams) error {
 	log.Infof("Running postdeploy actions for Nokia SR Linux '%s' node", s.Cfg.ShortName)
 
-	// add the ips as SANs
-	for _, ip := range []string{s.Cfg.MgmtIPv4Address, s.Cfg.MgmtIPv6Address} {
-		if ip != "" {
-			s.Cfg.SANs = append(s.Cfg.SANs, ip)
-		}
-	}
-
 	// generate the certificate
 	certificate, err := s.LoadOrGenerateCertificate(s.cert, s.topologyName)
 	if err != nil {
@@ -558,7 +551,7 @@ func (n *srl) addDefaultConfig(ctx context.Context) error {
 		DNSServers: n.Config().DNS.Servers,
 	}
 
-	n.setVersionSpecificParams(&tplData, n.swVersion)
+	n.setVersionSpecificParams(&tplData)
 
 	n.setCustomPrompt(&tplData)
 
@@ -818,7 +811,7 @@ gpgcheck=0`
 // setVersionSpecificParams sets version specific parameters in the template data struct
 // to enable/disable version-specific configuration blocks in the config template
 // or prepares data to conform to the expected format per specific version.
-func (n *srl) setVersionSpecificParams(tplData *srlTemplateData, swVersion *SrlVersion) {
+func (n *srl) setVersionSpecificParams(tplData *srlTemplateData) {
 	v := n.swVersion.String()
 
 	// in srlinux >= v23.10+ linuxadmin and admin user ssh keys can only be configured via the cli

diff --git a/schemas/clab.schema.json b/schemas/clab.schema.json
@@ -26,15 +26,6 @@
                         "IfNotPresent"
                     ]
                 },
-                "SANs": {
-                    "type": "array",
-                    "description": "list of subject alternative names (SAN) to use for this node",
-                    "markdownDescription": "list of [subject alternative names](https://containerlab.dev/manual/nodes/#subject-alternative-names) to use for this node",
-                    "items": {
-                        "type": "string"
-                    },
-                    "uniqueItems": true
-                },
                 "kind": {
                     "type": "string",
                     "description": "kind of this node",
@@ -484,6 +475,25 @@
                 "issue": {
                     "description": "Set to `true` to generate a TLS certificate for the node",
                     "markdownDescription": "Set to `true` to [generate a TLS certificate for the node](https://containerlab.dev/manual/nodes/#certificate)"
+                },
+                "sans": {
+                    "type": "array",
+                    "description": "list of subject alternative names (SAN) to use for this node",
+                    "markdownDescription": "list of [subject alternative names](https://containerlab.dev/manual/nodes/#subject-alternative-names) to use for this node",
+                    "items": {
+                        "type": "string"
+                    },
+                    "uniqueItems": true
+                },
+                "key-size": {
+                    "type": "integer",
+                    "description": "size of the to be generated key",
+                    "markdownDescription": "size of the to be generated key"
+                },
+                "validity-duration": {
+                    "type": "string",
+                    "description": "Duration for how long the certificate issued by the CA will be valid.",
+                    "markdownDescription": "Duration for how long the certificate issued by the CA will be valid."
                 }
             }
         },

diff --git a/tests/01-smoke/10-ca-parameter.robot b/tests/01-smoke/10-ca-parameter.robot
@@ -98,6 +98,11 @@ Verify l1 Certificate Validity
     ...    openssl x509 -in ${l1-cert} -text
     Check Certificat Validity Duration    ${certificate_output}    ${l1-validity-duration}
 
+Verify l2 extra SANs
+    ${rc}    ${certificate_output} =    Run And Return Rc And Output
+    ...    openssl x509 -in ${l2-cert} -text
+    Should Contain    ${certificate_output}    DNS:my.text.fqdn
+    Should Contain    ${certificate_output}    IP Address:192.168.33.44
 
 *** Keywords ***
 Teardown

diff --git a/tests/01-smoke/10-internal-ca.clab.yml b/tests/01-smoke/10-internal-ca.clab.yml
@@ -27,6 +27,9 @@ topology:
       certificate:
         issue: true
         key-size: 1024
+        sans:
+          - 192.168.33.44
+          - my.text.fqdn
     l3:
       kind: linux
       image: alpine:3

diff --git a/types/node_definition.go b/types/node_definition.go
@@ -29,8 +29,6 @@ type NodeDefinition struct {
 	Position              string            `yaml:"position,omitempty"`
 	Entrypoint            string            `yaml:"entrypoint,omitempty"`
 	Cmd                   string            `yaml:"cmd,omitempty"`
-	// list of subject Alternative Names (SAN) to be added to the node's certificate
-	SANs []string `yaml:"SANs,omitempty"`
 	// list of commands to run in container
 	Exec []string `yaml:"exec,omitempty"`
 	// list of bind mount compatible strings
@@ -354,13 +352,6 @@ func (n *NodeDefinition) GetExtras() *Extras {
 	return n.Extras
 }
 
-func (n *NodeDefinition) GetSANs() []string {
-	if n == nil {
-		return nil
-	}
-	return n.SANs
-}
-
 func (n *NodeDefinition) GetWaitFor() []string {
 	if n == nil {
 		return []string{}

diff --git a/types/topology.go b/types/topology.go
@@ -457,16 +457,6 @@ func (t *Topology) GetSysCtl(name string) map[string]string {
 	return nil
 }
 
-// GetSANs return the Subject Alternative Name configuration for the given node.
-func (t *Topology) GetSANs(name string) []string {
-	if ndef, ok := t.Nodes[name]; ok {
-		if len(ndef.GetSANs()) > 0 {
-			return ndef.GetSANs()
-		}
-	}
-	return nil
-}
-
 // GetNodeExtras returns the 'extras' section for the given node.
 func (t *Topology) GetNodeExtras(name string) *Extras {
 	if ndef, ok := t.Nodes[name]; ok {

diff --git a/types/types.go b/types/types.go
@@ -170,8 +170,6 @@ type NodeConfig struct {
 	// Extra /etc/hosts entries for all nodes.
 	ExtraHosts []string          `json:"extra-hosts,omitempty"`
 	Labels     map[string]string `json:"labels,omitempty"` // container labels
-	// List of Subject Alternative Names (SAN) to be added to the node's TLS certificate
-	SANs []string `json:"SANs,omitempty"`
 	// Ignite sandbox and kernel imageNames
 	Sandbox string `json:"sandbox,omitempty"`
 	Kernel  string `json:"kernel,omitempty"`
@@ -337,6 +335,8 @@ type CertificateConfig struct {
 	KeySize int `yaml:"key-size,omitempty"`
 	// ValidityDuration is the duration of the certificate validity
 	ValidityDuration time.Duration `yaml:"validity-duration"`
+	// list of subject Alternative Names (SAN) to be added to the node's certificate
+	SANs []string `yaml:"sans,omitempty"`
 }
 
 // Merge merges the given CertificateConfig into the current one.
@@ -357,6 +357,10 @@ func (c *CertificateConfig) Merge(x *CertificateConfig) *CertificateConfig {
 		c.KeySize = x.KeySize
 	}
 
+	if len(x.SANs) > 0 {
+		c.SANs = x.SANs
+	}
+
 	return c
 }