Support credential API

Implements the credential rotation feature introduced in Magnum. The
naming scheme of application credentials created has been changed to
include a nonce value to allow validation of the new credential before
deletion of the old one. Existing app credentials are now identified
by decoding their ID from the corresponding secret in the active
cluster.

Change-Id: Ibd01e145af498c4b2a8e38fb0faf48f36da0ab98
Signed-off-by: Matthew Northcott <matthewnorthcott@catalystcloud.nz>

magnum_capi_helm/common/app_creds.py

@@ -16,6 +16,7 @@
 # This code is making use of the good work done here:
 # https://github.com/vexxhost/magnum-cluster-api/blob/main/magnum_cluster_api/resources.py
 
+import secrets
 import yaml
 
 import certifi
@@ -23,6 +24,7 @@ import keystoneauth1
 from oslo_log import log as logging
 
 from magnum.common import clients
+from magnum.common import context as ctx
 from magnum.common import utils
 import magnum.conf
 
@@ -30,6 +32,17 @@ CONF = magnum.conf.CONF
 LOG = logging.getLogger(__name__)
 
 
+class ApplicationCredentialError(Exception):
+    message = "Failed to perform action on application credential."
+
+    def __init__(self, message=None):
+        self.message = message or self.message
+        super(ApplicationCredentialError, self).__init__(self.message)
+
+    def __str__(self):
+        return self.message
+
+
 def _get_openstack_ca_certificate():
     # This function returns the CA bundle to use when verifying TLS
     # connections to the OpenStack API in both the Cluster API provider
@@ -47,16 +60,20 @@ def _get_openstack_ca_certificate():
     return ca_certificate
 
 
-def _create_app_cred(context, cluster):
+def create_app_cred(context, cluster):
     osc = clients.OpenStackClients(context)
     # TODO(johngarbutt) be sure not to allow the admin role
     # roles = [role for role in context.roles if role != "admin"]
-    app_cred = osc.keystone().client.application_credentials.create(
-        user=cluster.user_id,
-        name=f"magnum-{cluster.uuid}",
-        description=f"Magnum cluster ({cluster.uuid})",
+    return osc.keystone().client.application_credentials.create(
+        user=context.user_id,
+        name=f"magnum-{cluster.uuid}-{secrets.token_hex(4)}",
+        description=f"Magnum cluster ({cluster.name or cluster.uuid})",
         # roles=roles,
     )
+
+
+def _get_app_cred_clouds_dict(context, app_cred):
+    osc = clients.OpenStackClients(context)
     return {
         "clouds": {
             "openstack": {
@@ -79,26 +96,34 @@ def _create_app_cred(context, cluster):
     }
 
 
-def get_app_cred_string_data(context, cluster):
-    app_cred_dict = _create_app_cred(context, cluster)
-    clouds_yaml_str = yaml.safe_dump(app_cred_dict)
+def get_app_cred_string_data(context, app_cred):
+    clouds_dict = _get_app_cred_clouds_dict(context, app_cred)
     return {
         "cacert": _get_openstack_ca_certificate(),
-        "clouds.yaml": clouds_yaml_str,
+        "clouds.yaml": yaml.safe_dump(clouds_dict),
     }
 
 
-def delete_app_cred(context, cluster):
+def delete_app_cred(cluster, app_cred_id):
+    # NOTE(northcottmt): Admin privileges are needed to delete app creds
+    # outside the requestor scope
+    context = ctx.make_admin_context()
     osc = clients.OpenStackClients(context)
+    kst = osc.keystone()
+
+    LOG.debug(
+        f"Deleting application credential with ID {app_cred_id} "
+        f"for cluster {cluster.uuid}"
+    )
+
     try:
-        appcred = osc.keystone().client.application_credentials.find(
-            name=f"magnum-{cluster.uuid}", user=cluster.user_id
-        )
+        app_cred = kst.client.application_credentials.get(app_cred_id)
     except keystoneauth1.exceptions.http.NotFound:
-        # We don't want this to be a failure condition as it may prevent
-        # cleanup of broken clusters, e.g. if cluster creation fails
-        # before the appcred is created or cluster deletion fails after
-        # the appcred is deleted
-        LOG.warning("Appcred does not exist for %s", cluster.uuid)
-    else:
-        appcred.delete()
+        raise ApplicationCredentialError(f"{app_cred_id} does not exist.")
+
+    if not app_cred.name.startswith(f"magnum-{cluster.uuid}"):
+        raise ApplicationCredentialError(
+            f"{app_cred_id} is not managed by Magnum."
+        )
+
+    app_cred.delete()

magnum_capi_helm/driver.py

@@ -12,6 +12,9 @@
 import enum
 import re
 
+import requests
+import yaml
+
 from magnum.api import utils as api_utils
 from magnum.common import clients
 from magnum.common import exception
@@ -326,6 +329,9 @@ class Driver(driver.Driver):
         cluster.save()
 
     def _update_status_deleting(self, context, cluster):
+        # Fetch the current app cred ID before we delete all secrets
+        app_cred_id = self._get_app_cred_id(cluster)
+
         # Once the Cluster API cluster is gone, we need to clean up
         # the secrets we created
         self._k8s_client.delete_all_secrets_by_label(
@@ -334,8 +340,23 @@ class Driver(driver.Driver):
             driver_utils.cluster_namespace(cluster),
         )
 
-        # We also need to clean up the appcred that we made
-        app_creds.delete_app_cred(context, cluster)
+        cluster.status_reason = None
+
+        # Clean up the app cred
+        if app_cred_id:
+            try:
+                app_creds.delete_app_cred(cluster, app_cred_id)
+            except app_creds.ApplicationCredentialError as e:
+                # NOTE(northcottmt): We don't want this to be a failure
+                # condition as it may prevent cleanup of broken clusters,
+                # e.g. if cluster creation fails before the appcred is created
+                # or cluster deletion fails after the appcred is deleted
+                error_msg = (
+                    "Failed to delete application credential for "
+                    f"cluster {cluster.uuid}: {e}"
+                )
+                cluster.status_reason = error_msg
+                LOG.warning(error_msg)
 
         cluster.status = fields.ClusterStatus.DELETE_COMPLETE
         cluster.save()
@@ -431,8 +452,9 @@ class Driver(driver.Driver):
         }
 
     def _create_appcred_secret(self, context, cluster):
-        string_data = app_creds.get_app_cred_string_data(context, cluster)
-        name = self._get_app_cred_name(cluster)
+        app_cred = app_creds.create_app_cred(context, cluster)
+        string_data = app_creds.get_app_cred_string_data(context, app_cred)
+        name = self._get_app_cred_secret_name(cluster)
         self._k8s_client.apply_secret(
             name,
             {
@@ -441,6 +463,7 @@ class Driver(driver.Driver):
             },
             driver_utils.cluster_namespace(cluster),
         )
+        return app_cred
 
     def _ensure_certificate_secrets(self, context, cluster):
         # Magnum creates CA certs for each of the Kubernetes components that
@@ -531,9 +554,33 @@ class Driver(driver.Driver):
             self._get_os_distro(image),
         )
 
-    def _get_app_cred_name(self, cluster):
+    def _get_app_cred_secret_name(self, cluster):
         return driver_utils.get_k8s_resource_name(cluster, "cloud-credentials")
 
+    def _get_app_cred_id(self, cluster):
+        # determine the existing application credential secret
+        secret_name = self._get_app_cred_secret_name(cluster)
+        secret_namespace = driver_utils.cluster_namespace(cluster)
+
+        # fetch the existing secret and unpack the application credential ID
+        try:
+            clouds_dict_str = self._k8s_client.get_secret_value(
+                secret_name, secret_namespace, "clouds.yaml"
+            )
+            if clouds_dict_str:
+                clouds_dict = yaml.safe_load(clouds_dict_str)
+                auth_dict = clouds_dict["clouds"]["openstack"]["auth"]
+                return auth_dict["application_credential_id"]
+            else:
+                msg = "secret containing credential does not exist"
+        except (KeyError, yaml.YAMLError) as e:
+            msg = str(e)
+
+        LOG.error(
+            "Failed to fetch application credential for cluster "
+            f"{cluster.uuid}: {msg}"
+        )
+
     def _get_etcd_config(self, cluster):
         # Support new-style and legacy labels for volume size and type, with
         # new-style labels taking precedence
@@ -847,7 +894,9 @@ class Driver(driver.Driver):
             "kubernetesVersion": kube_version,
             "machineImageId": image_id,
             "machineSSHKeyName": cluster.keypair or None,
-            "cloudCredentialsSecretName": self._get_app_cred_name(cluster),
+            "cloudCredentialsSecretName": self._get_app_cred_secret_name(
+                cluster
+            ),
             "etcd": self._get_etcd_config(cluster),
             "apiServer": {
                 "associateFloatingIP": self._get_label_bool(
@@ -1154,6 +1203,101 @@ class Driver(driver.Driver):
             [ng for ng in cluster.nodegroups if ng.name != nodegroup.name],
         )
 
+    def rotate_credential(self, context, cluster):
+        # Current cluster owner to revert to if rotation fails
+        old_user_id = cluster.user_id
+
+        # Template cluster name for log messages
+        cluster_name = (
+            f"'{cluster.name}' ({cluster.uuid})"
+            if cluster.name
+            else cluster.uuid
+        )
+
+        LOG.info(f"Rotating application credential for cluster {cluster_name}")
+
+        # Set new owner now to ensure resource labels are applied correctly
+        cluster.user_id = context.user_id
+        cluster.status_reason = None
+
+        # Fetch the existing application credential ID to check against later
+        old_app_cred_id = self._get_app_cred_id(cluster)
+
+        # Tuple of possible exceptions raised when creating new app cred
+        exceptions = (
+            app_creds.ApplicationCredentialError,
+            exception.AuthorizationFailure,  # Raised on Keystone error
+            requests.exceptions.RequestException,
+            yaml.YAMLError,
+        )
+        error_msg = None
+
+        try:
+            # Create and apply application credential to cluster
+            new_app_cred = self._create_appcred_secret(context, cluster)
+        except exceptions as e:
+            error_msg = str(e)
+            LOG.warning(
+                "Failed to rotate application credential for cluster "
+                f"{cluster_name}: {e}"
+            )
+        else:
+            if not new_app_cred:
+                error_msg = "Application credential secret is unset."
+                LOG.critical(
+                    "Failed to rotate application credential for cluster "
+                    f"{cluster_name}: {error_msg}"
+                )
+            elif new_app_cred.id == old_app_cred_id:
+                error_msg = "Application credential secret failed to apply."
+                LOG.error(
+                    "Failed to rotate application credential for cluster "
+                    f"{cluster_name}: {error_msg}"
+                )
+            # Update user-id label on existing secrets if it has changed
+            elif cluster.user_id != old_user_id:
+                try:
+                    self._ensure_certificate_secrets(context, cluster)
+                except requests.exceptions.RequestException as e:
+                    error_msg = str(e)
+                    LOG.error(
+                        f"Failed to transfer ownership of cluster "
+                        f"{cluster_name} from {old_user_id} to "
+                        f"{cluster.user_id}: {e}"
+                    )
+                else:
+                    LOG.info(
+                        f"Ownership of cluster {cluster_name} transferred "
+                        f"from {cluster.user_id} to {context.user_id}"
+                    )
+
+        if error_msg:
+            cluster.status = fields.ClusterStatus.UPDATE_FAILED
+            cluster.status_reason = error_msg
+            cluster.user_id = old_user_id
+            cluster.save()
+            raise exception.MagnumException(message=error_msg)
+
+        # Remove the old credential now that the new one is working
+        if old_app_cred_id:
+            try:
+                app_creds.delete_app_cred(cluster, old_app_cred_id)
+            except app_creds.ApplicationCredentialError as e:
+                # NOTE(northcottmt): Not a failure condition as the cluster is
+                # functional, but report the old app cred could not be deleted
+                cluster.status_reason = (
+                    "A new application credential was applied successfully "
+                    "but the previous application credential could not be "
+                    f"deleted: {e}"
+                )
+                LOG.warning(
+                    "Failed to delete application credential for "
+                    f"cluster {cluster_name}: {e}"
+                )
+
+        cluster.status = fields.ClusterStatus.UPDATE_COMPLETE
+        cluster.save()
+
     def create_federation(self, context, federation):
         raise NotImplementedError("Will not implement 'create_federation'")
 

magnum_capi_helm/kubernetes.py

@@ -155,6 +155,16 @@ class Client(requests.Session):
     def delete_all_secrets_by_label(self, label, value, namespace):
         Secret(self).delete_all_by_label(label, value, namespace)
 
+    def get_secret(self, secret_name, namespace):
+        return Secret(self).fetch(secret_name, namespace)
+
+    def get_secret_value(self, secret_name, namespace, key):
+        secret = self.get_secret(secret_name, namespace)
+        if secret:
+            encoded_value = secret["data"][key].encode()
+            decoded_value = base64.b64decode(encoded_value).decode()
+            return decoded_value
+
     def get_capi_cluster(self, name, namespace):
         return Cluster(self).fetch(name, namespace)
 

magnum_capi_helm/tests/common/test_app_creds.py

@@ -47,17 +47,24 @@ class TestAppCreds(base.DbTestCase):
 
         self.assertIsNotNone(cert)
 
+    @mock.patch("secrets.token_hex")
     @mock.patch.object(clients, "OpenStackClients")
-    def test_create_app_cred(self, mock_client):
+    def test_create_app_cred(self, mock_client, mock_token):
         mock_client().cinder_region_name.return_value = "cinder"
         mock_client().url_for.return_value = "http://keystone"
         mock_app_cred = mock_client().keystone().client.application_credentials
         app_cred = collections.namedtuple("appcred", ["id", "secret"])
         mock_app_cred.create.return_value = app_cred("id", "pass")
         context = mock.MagicMock()
+        context.user_id = "fake_user"
         context.roles = ["member", "foo", "admin"]
 
-        app_cred = app_creds._create_app_cred(context, self.cluster_obj)
+        mock_token_hex_nonce = "abcd1234"
+        mock_token.return_value = mock_token_hex_nonce
+        app_cred = app_creds.create_app_cred(context, self.cluster_obj)
+        app_cred_string_data = app_creds._get_app_cred_clouds_dict(
+            context, app_cred
+        )
 
         expected = {
             "clouds": {
@@ -75,28 +82,38 @@ class TestAppCreds(base.DbTestCase):
                 }
             }
         }
-        self.assertEqual(expected, app_cred)
+        self.assertEqual(expected, app_cred_string_data)
         mock_client().url_for.assert_called_once_with(
             service_type="identity", interface="public"
         )
         mock_app_cred.create.assert_called_once_with(
-            user="fake_user",
-            name=f"magnum-{self.cluster_obj.uuid}",
-            description=f"Magnum cluster ({self.cluster_obj.uuid})",
+            user=context.user_id,
+            name=f"magnum-{self.cluster_obj.uuid}-{mock_token_hex_nonce}",
+            description="Magnum cluster "
+            + f"({self.cluster_obj.name or self.cluster_obj.uuid})",
             # roles=["member", "foo"],
         )
 
     @mock.patch.object(app_creds, "_get_openstack_ca_certificate")
-    @mock.patch.object(app_creds, "_create_app_cred")
-    def test_get_app_cred_yaml(self, mock_create, mock_ca):
-        mock_ca.return_value = "cacert"
-        mock_create.return_value = {
+    @mock.patch.object(app_creds, "_get_app_cred_clouds_dict")
+    def test_get_app_cred_yaml(self, mock_clouds, mock_ca):
+        app_cred = collections.namedtuple("appcred", ["id", "secret"])
+        mock_app_cred = app_cred("id", "secret")
+        mock_clouds.return_value = {
             "clouds": {
-                "openstack": {"auth": {"application_credential_id": "id"}}
+                "openstack": {
+                    "auth": {"application_credential_id": mock_app_cred.id},
+                }
             }
         }
 
-        app_cred = app_creds.get_app_cred_string_data("context", "cluster")
+        mock_ca.return_value = "cacert"
+        string_data = app_creds.get_app_cred_string_data(
+            "context", mock_app_cred
+        )
+
+        mock_clouds.assert_called_once_with("context", mock_app_cred)
+        mock_ca.assert_called_once_with()
 
         expected = {
             "cacert": "cacert",
@@ -107,30 +124,68 @@ clouds:
       application_credential_id: id
 """,
         }
-        self.assertEqual(expected, app_cred)
+        self.assertEqual(expected, string_data)
 
     @mock.patch.object(clients, "OpenStackClients")
     def test_delete_app_cred(self, mock_client):
-        mock_app_cred = mock_client().keystone().client.application_credentials
-        mock_find = mock.MagicMock()
-        mock_app_cred.find.return_value = mock_find
-
-        app_creds.delete_app_cred("context", self.cluster_obj)
-
-        mock_find.delete.assert_called_once_with()
-        mock_app_cred.find.assert_called_once_with(
-            name=f"magnum-{self.cluster_obj.uuid}",
-            user="fake_user",
+        mock_app_cred_client = (
+            mock_client().keystone().client.application_credentials
         )
+        mock_app_cred = mock.MagicMock()
+        mock_app_cred.name.startswith.return_value = True
+        mock_app_cred_client.get.return_value = mock_app_cred
+
+        app_cred_id = "abcdef12345"
+        app_creds.delete_app_cred(self.cluster_obj, app_cred_id)
+
+        mock_app_cred.delete.assert_called_once()
+        mock_app_cred.name.startswith.assert_called_once_with(
+            f"magnum-{self.cluster_obj.uuid}"
+        )
+        mock_app_cred_client.get.assert_called_once_with(app_cred_id)
 
     @mock.patch.object(clients, "OpenStackClients")
     def test_delete_app_cred_not_found(self, mock_client):
-        mock_app_cred = mock_client().keystone().client.application_credentials
-        mock_app_cred.find.side_effect = keystoneauth1.exceptions.http.NotFound
-
-        app_creds.delete_app_cred("context", self.cluster_obj)
-
-        mock_app_cred.find.assert_called_once_with(
-            name=f"magnum-{self.cluster_obj.uuid}",
-            user="fake_user",
+        mock_app_cred_client = (
+            mock_client().keystone().client.application_credentials
         )
+        mock_app_cred_client.get.side_effect = (
+            keystoneauth1.exceptions.http.NotFound
+        )
+
+        app_cred_id = "abcdef12345"
+
+        self.assertRaises(
+            app_creds.ApplicationCredentialError,
+            app_creds.delete_app_cred,
+            self.cluster_obj,
+            app_cred_id,
+        )
+
+        mock_app_cred_client.get.assert_called_once_with(app_cred_id)
+
+    @mock.patch.object(clients, "OpenStackClients")
+    def test_delete_app_cred_invalid_name(self, mock_client):
+        mock_app_cred_client = (
+            mock_client().keystone().client.application_credentials
+        )
+        mock_app_cred = mock.MagicMock()
+        mock_app_cred_name = mock.MagicMock()
+        mock_app_cred_name.startswith.return_value = False
+        mock_app_cred.name = mock_app_cred_name
+        mock_app_cred_client.get.return_value = mock_app_cred
+
+        app_cred_id = "abcdef12345"
+
+        self.assertRaises(
+            app_creds.ApplicationCredentialError,
+            app_creds.delete_app_cred,
+            self.cluster_obj,
+            app_cred_id,
+        )
+
+        mock_app_cred.delete.assert_not_called()
+        mock_app_cred.name.startswith.assert_called_once_with(
+            f"magnum-{self.cluster_obj.uuid}"
+        )
+        mock_app_cred_client.get.assert_called_once_with(app_cred_id)

magnum_capi_helm/tests/test_driver.py

@@ -592,14 +592,17 @@ class ClusterAPIDriverTest(base.DbTestCase):
 
     @mock.patch.object(app_creds, "delete_app_cred")
     @mock.patch.object(kubernetes.Client, "load")
-    def test_update_status_deleting(self, mock_load, mock_delete):
+    @mock.patch.object(driver.Driver, "_get_app_cred_id")
+    def test_update_status_deleting(self, mock_get, mock_load, mock_delete):
         mock_client = mock.MagicMock(spec=kubernetes.Client)
+        app_cred_id = "abc123"
+        mock_get.return_value = app_cred_id
         mock_load.return_value = mock_client
 
         self.driver._update_status_deleting(self.context, self.cluster_obj)
 
         self.assertEqual("DELETE_COMPLETE", self.cluster_obj.status)
-        mock_delete.assert_called_once_with(self.context, self.cluster_obj)
+        mock_delete.assert_called_once_with(self.cluster_obj, app_cred_id)
         mock_client.delete_all_secrets_by_label.assert_called_once_with(
             "magnum.openstack.org/cluster-uuid",
             self.cluster_obj.uuid,
@@ -1911,9 +1914,10 @@ class ClusterAPIDriverTest(base.DbTestCase):
             self.driver, self.context, self.cluster_obj
         )
 
+    @mock.patch.object(app_creds, "create_app_cred")
     @mock.patch.object(app_creds, "get_app_cred_string_data")
     @mock.patch.object(kubernetes.Client, "load")
-    def test_create_appcred_secret(self, mock_load, mock_sd):
+    def test_create_appcred_secret(self, mock_load, mock_sd, mock_create):
         mock_client = mock.MagicMock(spec=kubernetes.Client)
         mock_load.return_value = mock_client
         mock_sd.return_value = {"cacert": "ca", "clouds.yaml": "appcred"}
@@ -3295,3 +3299,201 @@ class ClusterAPIDriverTest(base.DbTestCase):
             "loadBalancerProvider": "amphora",
         }
         self.assertEqual(apiserver_expected, helm_install_values["apiServer"])
+
+    @mock.patch.object(app_creds, "delete_app_cred", autospec=True)
+    @mock.patch.object(
+        driver.Driver, "_ensure_certificate_secrets", autospec=True
+    )
+    @mock.patch.object(driver.Driver, "_create_appcred_secret", autospec=True)
+    @mock.patch.object(driver.Driver, "_get_app_cred_id", autospec=True)
+    def test_rotate_credential(
+        self, mock_get, mock_create, mock_certs, mock_delete
+    ):
+        app_cred_id_old = "abcde12345"
+        app_cred_id_new = "edcba54321"
+
+        mock_app_cred = mock.MagicMock()
+        mock_app_cred.id = app_cred_id_new
+
+        mock_get.return_value = app_cred_id_old
+        mock_create.return_value = mock_app_cred
+
+        self.driver.rotate_credential(self.context, self.cluster_obj)
+
+        mock_get.assert_called_once_with(self.driver, self.cluster_obj)
+        mock_create.assert_called_once_with(
+            self.driver, self.context, self.cluster_obj
+        )
+        mock_certs.assert_not_called()  # user_id unchanged
+        mock_delete.assert_called_once_with(self.cluster_obj, app_cred_id_old)
+
+        self.assertEqual(self.context.user_id, self.cluster_obj.user_id)
+        self.assertEqual(
+            fields.ClusterStatus.UPDATE_COMPLETE, self.cluster_obj.status
+        )
+        self.assertIsNone(self.cluster_obj.status_reason)
+
+    @mock.patch.object(app_creds, "delete_app_cred", autospec=True)
+    @mock.patch.object(
+        driver.Driver, "_ensure_certificate_secrets", autospec=True
+    )
+    @mock.patch.object(driver.Driver, "_create_appcred_secret", autospec=True)
+    @mock.patch.object(driver.Driver, "_get_app_cred_id", autospec=True)
+    def test_rotate_credential_new_user(
+        self, mock_get, mock_create, mock_certs, mock_delete
+    ):
+        self.cluster_obj.user_id = "user_a"
+        self.context.user_id = "user_b"
+
+        app_cred_id_old = "abcde12345"
+        app_cred_id_new = "edcba54321"
+
+        mock_app_cred = mock.MagicMock()
+        mock_app_cred.id = app_cred_id_new
+
+        mock_get.return_value = app_cred_id_old
+        mock_create.return_value = mock_app_cred
+
+        self.driver.rotate_credential(self.context, self.cluster_obj)
+
+        mock_get.assert_called_once_with(self.driver, self.cluster_obj)
+        mock_create.assert_called_once_with(
+            self.driver, self.context, self.cluster_obj
+        )
+        mock_certs.assert_called_once_with(
+            self.driver, self.context, self.cluster_obj
+        )
+        mock_delete.assert_called_once_with(self.cluster_obj, app_cred_id_old)
+
+        self.assertEqual(
+            fields.ClusterStatus.UPDATE_COMPLETE, self.cluster_obj.status
+        )
+        self.assertIsNone(self.cluster_obj.status_reason)
+
+    @mock.patch.object(app_creds, "delete_app_cred", autospec=True)
+    @mock.patch.object(
+        driver.Driver, "_ensure_certificate_secrets", autospec=True
+    )
+    @mock.patch.object(driver.Driver, "_create_appcred_secret", autospec=True)
+    @mock.patch.object(driver.Driver, "_get_app_cred_id", autospec=True)
+    def test_rotate_credential_delete_failed(
+        self, mock_get, mock_create, mock_certs, mock_delete
+    ):
+        app_cred_id = "abcde12345"
+
+        mock_app_cred = mock.MagicMock()
+        mock_app_cred.id = app_cred_id
+
+        mock_get.return_value = None
+        mock_create.return_value = mock_app_cred
+
+        self.driver.rotate_credential(self.context, self.cluster_obj)
+
+        mock_get.assert_called_once_with(self.driver, self.cluster_obj)
+        mock_create.assert_called_once_with(
+            self.driver, self.context, self.cluster_obj
+        )
+        mock_certs.assert_not_called()  # user_id unchanged
+        mock_delete.assert_not_called()
+
+        # Failure to delete the old app cred is not a error condition
+        self.assertEqual(self.context.user_id, self.cluster_obj.user_id)
+        self.assertEqual(
+            fields.ClusterStatus.UPDATE_COMPLETE, self.cluster_obj.status
+        )
+        self.assertIsNone(self.cluster_obj.status_reason)
+
+    @mock.patch.object(app_creds, "delete_app_cred", autospec=True)
+    @mock.patch.object(
+        driver.Driver, "_ensure_certificate_secrets", autospec=True
+    )
+    @mock.patch.object(driver.Driver, "_create_appcred_secret", autospec=True)
+    @mock.patch.object(driver.Driver, "_get_app_cred_id", autospec=True)
+    def test_rotate_credential_create_failed(
+        self, mock_get, mock_create, mock_certs, mock_delete
+    ):
+        mock_create.side_effect = exception.AuthorizationFailure
+
+        self.assertRaises(
+            exception.MagnumException,
+            self.driver.rotate_credential,
+            self.context,
+            self.cluster_obj,
+        )
+
+        mock_get.assert_called_once_with(self.driver, self.cluster_obj)
+        mock_create.assert_called_once_with(
+            self.driver, self.context, self.cluster_obj
+        )
+        mock_certs.assert_not_called()
+        mock_delete.assert_not_called()
+
+        self.assertEqual(
+            fields.ClusterStatus.UPDATE_FAILED, self.cluster_obj.status
+        )
+        self.assertIsNotNone(self.cluster_obj.status_reason)
+
+    @mock.patch.object(app_creds, "delete_app_cred", autospec=True)
+    @mock.patch.object(
+        driver.Driver, "_ensure_certificate_secrets", autospec=True
+    )
+    @mock.patch.object(driver.Driver, "_create_appcred_secret", autospec=True)
+    @mock.patch.object(driver.Driver, "_get_app_cred_id", autospec=True)
+    def test_rotate_credential_is_none(
+        self, mock_get, mock_create, mock_certs, mock_delete
+    ):
+        mock_create.return_value = None
+
+        self.assertRaises(
+            exception.MagnumException,
+            self.driver.rotate_credential,
+            self.context,
+            self.cluster_obj,
+        )
+
+        mock_get.assert_called_once_with(self.driver, self.cluster_obj)
+        mock_create.assert_called_once_with(
+            self.driver, self.context, self.cluster_obj
+        )
+        mock_certs.assert_not_called()
+        mock_delete.assert_not_called()
+
+        self.assertEqual(
+            fields.ClusterStatus.UPDATE_FAILED, self.cluster_obj.status
+        )
+        self.assertIsNotNone(self.cluster_obj.status_reason)
+
+    @mock.patch.object(app_creds, "delete_app_cred", autospec=True)
+    @mock.patch.object(
+        driver.Driver, "_ensure_certificate_secrets", autospec=True
+    )
+    @mock.patch.object(driver.Driver, "_create_appcred_secret", autospec=True)
+    @mock.patch.object(driver.Driver, "_get_app_cred_id", autospec=True)
+    def test_rotate_credential_unchanged(
+        self, mock_get, mock_create, mock_certs, mock_delete
+    ):
+        mock_app_cred_id = "abcde12355"
+        mock_app_cred = mock.MagicMock()
+        mock_app_cred.id = mock_app_cred_id
+
+        mock_get.return_value = mock_app_cred_id
+        mock_create.return_value = mock_app_cred
+
+        self.assertRaises(
+            exception.MagnumException,
+            self.driver.rotate_credential,
+            self.context,
+            self.cluster_obj,
+        )
+
+        mock_get.assert_called_once_with(self.driver, self.cluster_obj)
+        mock_create.assert_called_once_with(
+            self.driver, self.context, self.cluster_obj
+        )
+        mock_certs.assert_not_called()
+        mock_delete.assert_not_called()
+
+        self.assertEqual(
+            fields.ClusterStatus.UPDATE_FAILED, self.cluster_obj.status
+        )
+        self.assertIsNotNone(self.cluster_obj.status_reason)

magnum_capi_helm/tests/test_kubernetes.py

@@ -211,6 +211,52 @@ class TestKubernetesClient(base.TestCase):
         )
         mock_response.raise_for_status.assert_called_once_with()
 
+    @mock.patch.object(requests.Session, "request")
+    def test_get_secret(self, mock_request):
+        client = kubernetes.Client(TEST_KUBECONFIG)
+        mock_response = mock.MagicMock()
+        mock_response.status_code = 200
+        mock_request.return_value = mock_response
+
+        secret_name = "secret1"
+        secret_namespace = "ns1"
+        client.get_secret(secret_name, secret_namespace)
+
+        mock_request.assert_called_once_with(
+            "GET",
+            "https://test:6443/api/v1/namespaces"
+            f"/{secret_namespace}/secrets/{secret_name}",
+            allow_redirects=True,
+        )
+
+    @mock.patch.object(requests.Session, "request")
+    def test_get_secret_value(self, mock_request):
+        client = kubernetes.Client(TEST_KUBECONFIG)
+        mock_response = mock.MagicMock()
+        mock_response.status_code = 200
+        mock_request.return_value = mock_response
+
+        secret_name = "secret1"
+        secret_namespace = "ns1"
+        secret_key = "mykey"
+        secret_value = "mysecretvalue"
+        mock_response.json.return_value = {
+            "data": {
+                secret_key: base64.b64encode(secret_value.encode()).decode(),
+            }
+        }
+
+        self.assertEqual(
+            client.get_secret_value(secret_name, secret_namespace, secret_key),
+            secret_value,
+        )
+        mock_request.assert_called_once_with(
+            "GET",
+            "https://test:6443/api/v1/namespaces"
+            f"/{secret_namespace}/secrets/{secret_name}",
+            allow_redirects=True,
+        )
+
     @mock.patch.object(requests.Session, "request")
     def test_get_capi_cluster_found(self, mock_request):
         client = kubernetes.Client(TEST_KUBECONFIG)

releasenotes/notes/credential-rotation-91773703e1f8f057.yaml

@@ -0,0 +1,12 @@
+---
+features:
+  - |
+    Adds support for application credential rotation for workload clusters
+    via the credential API.
+fixes:
+  - |
+    Fixes a bug where the application credential created for a cluster could
+    persist after cluster deletion if the performing user did not have the
+    correct privileges to delete it. This only applies to clusters using
+    Keystone Ussuri or later due to a bug in earlier releases. See `LP#1901207
+    <https://bugs.launchpad.net/keystone/+bug/1901207>`__