Remap image_pull_policy field on Container from ImagePullPolicy…

… enum to `str` (#877)

**Pull Request Checklist**
- [x] Fixes #784
- [x] Tests added
- [x] Documentation/examples added
- [x] [Good commit messages](https://cbea.ms/git-commit/) and/or PR
title

**Description of PR**
When an autogenerated object is created through the service the service
returns auto generated objects. This means the services relies on
autogenerated code _only_. #784 documents how creating an autogen
`Workflow` object fails because the value of `Always` does not fit the
`Template.container.imagePullPolicy` field. This happens because the
returned value is of type `str` (from the Argo Server) while the Python
value is an enum of type `ImagePullPolicy`. This is caused by an
inconsistency in the OpenAPI specification. Some fields use `str` for
image pull policy (look at Events, User Container, etc) while Container
uses `ImagePullPolicy`. This PR changes the field in the OpenAPI schema
specification so that this Container field is also a `str`. This should
solve #784. Our Pydantic configuration is set to use enum values, which
means that users who rely on the autogen Container object and pass the
ImagePullPolicy object in will have Pydantic automatically extract the
`.value` field of that enum. The Hera objects already accommodate both
str and ImagePullPolicy, so no breaking change there.

---------

Signed-off-by: Flaviu Vadan <flaviuvadan@gmail.com>

scripts/spec.py

@@ -3,7 +3,7 @@
 import json
 import logging
 import sys
-from typing import Dict, List, Set, Tuple
+from typing import Dict, List, Optional, Set, Tuple, Union
 
 import requests
 
@@ -56,17 +56,26 @@
 # these are specifications of objects with fields that are marked to have a union type of IntOrString. However, K8s
 # only accepts one or the other, unfortunately. Here, we remap those fields from their respective `$ref`s, which
 # contain a specific type, to another type. The mapping is from the `$ref` to a tuple of the existing type and the
-# new type. The dictionary model is:
+# new type. While the first piece of the mapping tuple must be defined the second one is optional - if the second
+# part of the tuple is None then the field will be removed. The dictionary model is:
 # { object name: { field name: ( ( existing field, existing value ) , ( new field, new value ) ) } }
-INT_OR_STRING_FIELD_REMAPPING: Dict[str, Dict[str, Tuple[Tuple[str, str], Tuple[str, str]]]] = {
+FIELD_REMAPPINGS: Dict[
+    str, Dict[str, Tuple[Tuple[str, Union[str, List[str]]], Optional[Tuple[str, Union[str, List[str]]]]]]
+] = {
     "io.k8s.api.core.v1.HTTPGetAction": {
         "port": (
             ("$ref", "#/definitions/io.k8s.apimachinery.pkg.util.intstr.IntOrString"),
             ("type", "integer"),
         ),
     },
+    "io.k8s.api.core.v1.Container": {
+        "imagePullPolicy": (
+            ("enum", None),
+            None,
+        )
+    },
 }
-for obj_name, field in INT_OR_STRING_FIELD_REMAPPING.items():
+for obj_name, field in FIELD_REMAPPINGS.items():
     try:
         curr_field = spec["definitions"][obj_name]
     except KeyError as e:
@@ -94,7 +103,12 @@
 
         # get the tuple of the existing field and value, and the new field and value
         existing_field, existing_value = field[property_to_change][0]
-        new_field, new_value = field[property_to_change][1]
+        if field[property_to_change][1] is None:
+            # if the second one is absent it means we want to delete the existing field
+            del curr_property[existing_field]
+            continue
+        else:
+            new_field, new_value = field[property_to_change][1]
 
         # check that the existing field and value are the same as the current field and value
         assert curr_property[existing_field] == existing_value, (
@@ -108,6 +122,21 @@
         if existing_field != new_field:
             del curr_property[existing_field]
 
+# there are also some specifications that have to be introduced manually for backwards compatibility purposes. This
+# block allows us to define those specifications and add them to the spec.
+MANUAL_SPECIFICATIONS: List[Tuple[str, Dict]] = [
+    (
+        "io.k8s.api.core.v1.ImagePullPolicy",
+        {
+            "description": "An enum that contains available image pull policy options.",
+            "type": "string",
+            "enum": ["Always", "Never", "IfNotPresent"],
+        },
+    ),
+]
+for obj_name, obj_spec in MANUAL_SPECIFICATIONS:
+    spec["definitions"][obj_name] = obj_spec
+
 # finally, we write the spec to the output file that is passed to use assuming the client wants to perform
 # something with this file
 with open(output_file, "w+") as f:

src/hera/events/models/io/k8s/api/core/v1.py

@@ -141,12 +141,6 @@ class ConfigMapKeySelector(BaseModel):
     )
 
 
-class ImagePullPolicy(Enum):
-    always = "Always"
-    if_not_present = "IfNotPresent"
-    never = "Never"
-
-
 class TerminationMessagePolicy(Enum):
     fallback_to_logs_on_error = "FallbackToLogsOnError"
     file = "File"
@@ -1240,6 +1234,12 @@ class WindowsSecurityContextOptions(BaseModel):
     )
 
 
+class ImagePullPolicy(Enum):
+    always = "Always"
+    never = "Never"
+    if_not_present = "IfNotPresent"
+
+
 class CSIVolumeSource(BaseModel):
     driver: str = Field(
         ...,
@@ -2828,7 +2828,7 @@ class Container(BaseModel):
             " StatefulSets."
         ),
     )
-    image_pull_policy: Optional[ImagePullPolicy] = Field(
+    image_pull_policy: Optional[str] = Field(
         default=None,
         alias="imagePullPolicy",
         description=(

src/hera/events/models/io/k8s/api/core/v1.pyi

@@ -39,11 +39,6 @@ class ConfigMapKeySelector(BaseModel):
     name: Optional[str]
     optional: Optional[bool]
 
-class ImagePullPolicy(Enum):
-    always: str
-    if_not_present: str
-    never: str
-
 class TerminationMessagePolicy(Enum):
     fallback_to_logs_on_error: str
     file: str
@@ -313,6 +308,11 @@ class WindowsSecurityContextOptions(BaseModel):
     host_process: Optional[bool]
     run_as_user_name: Optional[str]
 
+class ImagePullPolicy(Enum):
+    always: str
+    never: str
+    if_not_present: str
+
 class CSIVolumeSource(BaseModel):
     driver: str
     fs_type: Optional[str]
@@ -582,7 +582,7 @@ class Container(BaseModel):
     env: Optional[List[EnvVar]]
     env_from: Optional[List[EnvFromSource]]
     image: str
-    image_pull_policy: Optional[ImagePullPolicy]
+    image_pull_policy: Optional[str]
     lifecycle: Optional[Lifecycle]
     liveness_probe: Optional[Probe]
     name: Optional[str]

src/hera/workflows/models/io/k8s/api/core/v1.py

@@ -141,12 +141,6 @@ class ConfigMapKeySelector(BaseModel):
     )
 
 
-class ImagePullPolicy(Enum):
-    always = "Always"
-    if_not_present = "IfNotPresent"
-    never = "Never"
-
-
 class TerminationMessagePolicy(Enum):
     fallback_to_logs_on_error = "FallbackToLogsOnError"
     file = "File"
@@ -1240,6 +1234,12 @@ class WindowsSecurityContextOptions(BaseModel):
     )
 
 
+class ImagePullPolicy(Enum):
+    always = "Always"
+    never = "Never"
+    if_not_present = "IfNotPresent"
+
+
 class CSIVolumeSource(BaseModel):
     driver: str = Field(
         ...,
@@ -2828,7 +2828,7 @@ class Container(BaseModel):
             " StatefulSets."
         ),
     )
-    image_pull_policy: Optional[ImagePullPolicy] = Field(
+    image_pull_policy: Optional[str] = Field(
         default=None,
         alias="imagePullPolicy",
         description=(

src/hera/workflows/models/io/k8s/api/core/v1.pyi

@@ -39,11 +39,6 @@ class ConfigMapKeySelector(BaseModel):
     name: Optional[str]
     optional: Optional[bool]
 
-class ImagePullPolicy(Enum):
-    always: str
-    if_not_present: str
-    never: str
-
 class TerminationMessagePolicy(Enum):
     fallback_to_logs_on_error: str
     file: str
@@ -313,6 +308,11 @@ class WindowsSecurityContextOptions(BaseModel):
     host_process: Optional[bool]
     run_as_user_name: Optional[str]
 
+class ImagePullPolicy(Enum):
+    always: str
+    never: str
+    if_not_present: str
+
 class CSIVolumeSource(BaseModel):
     driver: str
     fs_type: Optional[str]
@@ -582,7 +582,7 @@ class Container(BaseModel):
     env: Optional[List[EnvVar]]
     env_from: Optional[List[EnvFromSource]]
     image: str
-    image_pull_policy: Optional[ImagePullPolicy]
+    image_pull_policy: Optional[str]
     lifecycle: Optional[Lifecycle]
     liveness_probe: Optional[Probe]
     name: Optional[str]