fix: skip validating inputs to allow updating credentials' name or description (#865)

Author: hsong-rh

src/aap_eda/api/serializers/eda_credential.py

@@ -11,7 +11,6 @@
 #  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 #  See the License for the specific language governing permissions and
 #  limitations under the License.
-
 from django.db.models import Q
 from drf_spectacular.utils import extend_schema_field
 from rest_framework import serializers
@@ -124,9 +123,13 @@ def validate(self, data):
             # for update
             credential_type = self.instance.credential_type
 
-        errors = validate_inputs(
-            credential_type.inputs, data.get("inputs", {})
-        )
+        inputs = data.get("inputs", {})
+
+        # allow emtpy inputs during updating
+        if self.partial and not bool(inputs):
+            return data
+
+        errors = validate_inputs(credential_type.inputs, inputs)
 
         if bool(errors):
             raise serializers.ValidationError(errors)

src/aap_eda/api/views/eda_credential.py

@@ -190,9 +190,10 @@ def partial_update(self, request, pk):
         )
         serializer.is_valid(raise_exception=True)
 
-        if serializer.validated_data.get("inputs"):
+        inputs = serializer.validated_data.get("inputs")
+        if inputs or inputs == {}:
             serializer.validated_data["inputs"] = inputs_to_store(
-                serializer.validated_data["inputs"],
+                inputs,
                 eda_credential.inputs,
             )
 

tests/integration/api/test_eda_credential.py

@@ -5,6 +5,7 @@
 from rest_framework.test import APIClient
 
 from aap_eda.core import enums, models
+from tests.integration.conftest import DUMMY_GPG_KEY
 from tests.integration.constants import api_url_v1
 
 DATA_DIR = Path(__file__).parent.parent.parent / "unit/data"
@@ -36,14 +37,18 @@
 }
 
 
+@pytest.mark.parametrize(
+    "inputs", [{}, {"username": "adam", "password": "secret"}]
+)
 @pytest.mark.django_db
 def test_create_eda_credential(
     client: APIClient,
     credential_type: models.CredentialType,
+    inputs,
 ):
     data_in = {
         "name": "eda-credential",
-        "inputs": {"username": "adam", "password": "secret"},
+        "inputs": inputs,
         "credential_type_id": credential_type.id,
     }
     response = client.post(f"{api_url_v1}/eda-credentials/", data=data_in)
@@ -111,24 +116,6 @@ def test_create_eda_credential_with_type(
     }
 
 
-@pytest.mark.django_db
-def test_create_eda_credential_with_empty_required_field(
-    client: APIClient, preseed_credential_types
-):
-    credential_type = models.CredentialType.objects.get(
-        name=enums.DefaultCredentialType.REGISTRY
-    )
-
-    data_in = {
-        "name": "eda-credential",
-        "inputs": {"host": ""},
-        "credential_type_id": credential_type.id,
-    }
-    response = client.post(f"{api_url_v1}/eda-credentials/", data=data_in)
-    assert response.status_code == status.HTTP_400_BAD_REQUEST
-    assert "Cannot be blank" in response.data["inputs.host"]
-
-
 @pytest.mark.parametrize(
     ("credential_type", "status_code", "key", "error_message"),
     [
@@ -349,7 +336,7 @@ def test_delete_managed_eda_credential(client: APIClient):
 
 
 @pytest.mark.django_db
-def test_partial_update_eda_credential(
+def test_partial_update_eda_credential_without_inputs(
     client: APIClient, credential_type: models.CredentialType
 ):
     obj = models.EdaCredential.objects.create(
@@ -371,8 +358,72 @@ def test_partial_update_eda_credential(
     }
 
 
+@pytest.mark.parametrize(
+    ("credential_type", "inputs"),
+    [
+        (enums.DefaultCredentialType.VAULT, {}),
+        (
+            enums.DefaultCredentialType.VAULT,
+            {"vault_password": "new_password"},
+        ),
+        (enums.DefaultCredentialType.AAP, {}),
+        (
+            enums.DefaultCredentialType.AAP,
+            {"host": "new_host", "username": "new user"},
+        ),
+        (enums.DefaultCredentialType.GPG, {}),
+        (enums.DefaultCredentialType.GPG, {"gpg_public_key": DUMMY_GPG_KEY}),
+        (enums.DefaultCredentialType.REGISTRY, {}),
+        (
+            enums.DefaultCredentialType.REGISTRY,
+            {"host": "new_host", "username": "new user", "verify_ssl": True},
+        ),
+        (enums.DefaultCredentialType.SOURCE_CONTROL, {}),
+        (
+            enums.DefaultCredentialType.SOURCE_CONTROL,
+            {"username": "new user", "password": "new password"},
+        ),
+    ],
+)
+@pytest.mark.django_db
+def test_partial_update_eda_credentials(
+    client: APIClient,
+    preseed_credential_types,
+    credential_type,
+    inputs,
+):
+    old_inputs = {"keep": "data"}
+    credential_type = models.CredentialType.objects.get(name=credential_type)
+    obj = models.EdaCredential.objects.create(
+        name="eda-credential",
+        inputs=old_inputs,
+        credential_type_id=credential_type.id,
+    )
+    new_name = "new-eda-credential"
+    new_description = "new-eda-credential description"
+    # update name, description with empty inputs
+    data = {"name": new_name, "description": new_description, "inputs": inputs}
+    response = client.patch(
+        f"{api_url_v1}/eda-credentials/{obj.id}/", data=data
+    )
+    assert response.status_code == status.HTTP_200_OK
+    result = response.data
+    assert result["name"] == new_name
+    assert result["description"] == new_description
+
+    data = {"inputs": inputs}
+    # update inputs
+    response = client.patch(
+        f"{api_url_v1}/eda-credentials/{obj.id}/", data=data
+    )
+    result = response.data
+    assert result["inputs"]["keep"] == old_inputs["keep"]
+    for key in inputs.keys():
+        assert result["inputs"][key] is not None
+
+
 @pytest.mark.django_db
-def test_partial_update_eda_credential_name(
+def test_partial_update_eda_credential_with_encrypted_output(
     client: APIClient, credential_type: models.CredentialType
 ):
     obj = models.EdaCredential.objects.create(