Skip to content

metalk8s_kubeconfig.validate() should not check kubeconfig mode #2958

Open
@alexandre-allard

Description

@alexandre-allard

Component: salt

Why this is needed:
If the mode of a certificate is changed, it will renew the certificate even if it is still valid.
Moreover, this function is also used in the metalk8s_watch_kubeconfig_expiry beacon where we don't want to check for this kind of things.

What should be done:
We should not check the mode in the validate() method, plus the mode should be configurable (and probably set to 0400 in the formulas using this state).

Implementation proposal (strongly recommended):
Remove mode check from the validate method and instead only rely on file.serialize in metalk8s_kubeconfig.managed state.
In salt/_states/kubeconfig.py, we should do something like (not tested):

--- a/salt/_states/kubeconfig.py
+++ b/salt/_states/kubeconfig.py
@@ -2,6 +2,8 @@

 from base64 import b64encode

+import yaml
+
 __virtualname__ = 'metalk8s_kubeconfig'


@@ -17,6 +19,7 @@ def managed(name,
             cluster,
             days_valid=365,
             days_remaining=90,
+            mode=None,
             **kwargs):
     """Generate kubeconfig file with identities for control plane components"""
     ret = {
@@ -47,21 +50,26 @@ def managed(name,
     # Validate if a kubeconfig already exists (idempotency)
     if __salt__['metalk8s_kubeconfig.validate'](
             name, b64_ca_cert, apiserver, user, days_remaining):
-        ret.update({'comment': 'kubeconfig file exists and is up-to-date'})
-        return ret
+        with open(name, 'r') as fd:
+            kubeconfig = yaml.safe_load(fd)
+        client_priv_key_b64 = kubeconfig['users'][0]['user']['client-key-data']
+        client_cert_b64 = kubeconfig['users'][0]['user']['client-certificate-data']

-    client_priv_key = __salt__['x509.create_private_key'](
-        text=True, verbose=False
-    )
+    else:
+        client_priv_key = __salt__['x509.create_private_key'](
+            text=True, verbose=False
+        )
+        client_priv_key_b64 = b64encode(client_priv_key.encode())

-    client_cert = __salt__['x509.create_certificate'](
-        text=True,
-        public_key=client_priv_key,  # pub key is sourced from priv key
-        ca_server=ca_server,
-        signing_policy=signing_policy,
-        days_valid=days_valid,
-        **client_cert_info
-    )
+        client_cert = __salt__['x509.create_certificate'](
+            text=True,
+            public_key=client_priv_key,  # pub key is sourced from priv key
+            ca_server=ca_server,
+            signing_policy=signing_policy,
+            days_valid=days_valid,
+            **client_cert_info
+        )
+        client_cert_b64 = b64encode(client_cert.encode())

     dataset = {
         'apiVersion': 'v1',
@@ -90,8 +98,8 @@ def managed(name,
             {
                 'name': user,
                 'user': {
-                    'client-certificate-data': b64encode(client_cert.encode()),
-                    'client-key-data': b64encode(client_priv_key.encode())
+                    'client-certificate-data': client_cert_b64,
+                    'client-key-data': client_priv_key_b64
                 }
             }
         ]
@@ -101,6 +109,6 @@ def managed(name,
         name=name,
         dataset=dataset,
         formatter="yaml",
-        mode="600",
+        mode=mode,
         makedirs=True
     )

So we always rely on file.serialize to handle changes.

Test plan:
We need to update Salt unit tests accordingly.

Metadata

Metadata

Assignees

No one assigned

    Labels

    complexity:easySomething that requires less than a day to fixkind:debtTechnical debt

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions