Merge pull request #3991 from Yelp/jfong/PAASTA-18095-serviceaccount-…

…or-iamrole

Disallow having service account + iam_role

paasta_tools/cli/schemas/kubernetes_schema.json

@@ -93,6 +93,14 @@
                             ]
                         }
                     ]
+                },
+                {
+                    "not": {
+                        "required": [
+                            "service_account_name",
+                            "iam_role"
+                        ]
+                    }
                 }
             ],
             "properties": {

paasta_tools/cli/schemas/tron_schema.json

@@ -17,6 +17,16 @@
             "required": [
                 "command"
             ],
+            "allOf": [
+                {
+                    "not": {
+                        "required": [
+                            "service_account_name",
+                            "iam_role"
+                        ]
+                    }
+                }
+            ],
             "properties": {
                 "name": {
                     "$ref": "#definitions/name"

tests/cli/test_cmds_validate.py

@@ -606,6 +606,37 @@ def test_instance_validate_schema_iam_role(
         assert expected_output in output
 
 
+@pytest.mark.parametrize(
+    "iam_role, service_account_name, instance_type, expected",
+    [
+        ("arn:aws:iam::12345678:role/some_role", None, "kubernetes", True),
+        ("arn:aws:iam::12345678:role/some_role", None, "eks", True),
+        ("arn:aws:iam::12345678:role/some_role", "some_svc_account", "eks", False),
+        (None, "some_svc_account", "eks", True),
+    ],
+)
+def test_instance_validate_schema_sa_and_iam_role(
+    iam_role,
+    service_account_name,
+    instance_type,
+    expected,
+    capsys,
+):
+    instance_content = f"""
+test_instance:
+  {"iam_role: "+iam_role if iam_role else ""}
+  {"service_account_name: "+service_account_name if service_account_name else ""}
+"""
+    with patch(
+        "paasta_tools.cli.cmds.validate.get_file_contents", autospec=True
+    ) as mock_get_file_contents:
+        mock_get_file_contents.return_value = instance_content
+        assert validate_schema("unused_service_path.yaml", instance_type) == expected
+        expected_output = SCHEMA_VALID if expected else SCHEMA_INVALID
+        output, _ = capsys.readouterr()
+        assert expected_output in output
+
+
 @patch("paasta_tools.cli.cmds.validate.get_file_contents", autospec=True)
 def test_tron_validate_schema_understands_underscores(mock_get_file_contents, capsys):
     tron_content = """
@@ -713,6 +744,40 @@ def test_tron_validate_schema_iam_role(iam_role, expected, capsys):
         assert expected_output in output
 
 
+@pytest.mark.parametrize(
+    "iam_role, service_account_name, expected",
+    [
+        ("arn:aws:iam::12345678:role/some_role", None, True),
+        ("arn:aws:iam::12345678:role/some_role", "some_svc_account", False),
+        (None, "some_svc_account", True),
+    ],
+)
+def test_tron_validate_schema_sa_and_iam_role(
+    iam_role,
+    service_account_name,
+    expected,
+    capsys,
+):
+    tron_content = f"""
+test_job:
+  node: paasta
+  schedule: "daily 04:00:00"
+  actions:
+    first:
+      {"iam_role: "+iam_role if iam_role else ""}
+      {"service_account_name: "+service_account_name if service_account_name else ""}
+      command: echo hello world
+"""
+    with patch(
+        "paasta_tools.cli.cmds.validate.get_file_contents", autospec=True
+    ) as mock_get_file_contents:
+        mock_get_file_contents.return_value = tron_content
+        assert validate_schema("unused_service_path.yaml", "tron") == expected
+        output, _ = capsys.readouterr()
+        expected_output = SCHEMA_VALID if expected else SCHEMA_INVALID
+        assert expected_output in output
+
+
 @pytest.mark.parametrize(
     "mock_content",
     (