From 6cba29548bf80b633ac7c832664a70b5876ea864 Mon Sep 17 00:00:00 2001 From: Flaviu Vadan Date: Thu, 23 Nov 2023 08:59:32 -0800 Subject: [PATCH] Add support for modifying field properties prior to model generation (#874) **Pull Request Checklist** - [x] Fixes #869 - [x] Tests added (we already have tests / examples that compare upstream and Hera YAMLs that include HTTPGetAction) - [x] Documentation/examples added - [x] [Good commit messages](https://cbea.ms/git-commit/) and/or PR title #869 reports how the field `IntOrString` incorrectly renders the port specification into a string that's not accepted by K8s. This PR adds a spec modification strategy that takes the remote Argo OpenAPI JSON specification and modifies the field from `IntOrString` to `integer`. This way the correct model is generated, and users know an integer is supposed to be used. Ideally, we'd fix these upstream but this at least unblocks current Hera users. --------- Signed-off-by: Flaviu Vadan --- .../workflows/upstream/daemon_nginx.md | 2 +- examples/workflows/upstream/daemon-nginx.yaml | 2 +- scripts/spec.py | 57 ++++++++++++++++++- src/hera/events/models/io/k8s/api/core/v1.py | 2 +- src/hera/events/models/io/k8s/api/core/v1.pyi | 2 +- .../workflows/models/io/k8s/api/core/v1.py | 2 +- .../workflows/models/io/k8s/api/core/v1.pyi | 2 +- 7 files changed, 62 insertions(+), 7 deletions(-) diff --git a/docs/examples/workflows/upstream/daemon_nginx.md b/docs/examples/workflows/upstream/daemon_nginx.md index c1bb2d4c4..707548854 100644 --- a/docs/examples/workflows/upstream/daemon_nginx.md +++ b/docs/examples/workflows/upstream/daemon_nginx.md @@ -66,7 +66,7 @@ The upstream example can be [found here](https://github.com/argoproj/argo-workfl readinessProbe: httpGet: path: / - port: '80' + port: 80 initialDelaySeconds: 2 timeoutSeconds: 1 daemon: true diff --git a/examples/workflows/upstream/daemon-nginx.yaml b/examples/workflows/upstream/daemon-nginx.yaml index a38fe5c27..92d5a6b0d 100644 --- a/examples/workflows/upstream/daemon-nginx.yaml +++ b/examples/workflows/upstream/daemon-nginx.yaml @@ -10,7 +10,7 @@ spec: readinessProbe: httpGet: path: / - port: '80' + port: 80 initialDelaySeconds: 2 timeoutSeconds: 1 daemon: true diff --git a/scripts/spec.py b/scripts/spec.py index 9ee4e86fc..b104847ea 100755 --- a/scripts/spec.py +++ b/scripts/spec.py @@ -3,7 +3,7 @@ import json import logging import sys -from typing import Dict, List, Set +from typing import Dict, List, Set, Tuple import requests @@ -53,6 +53,61 @@ ) spec["definitions"][definition]["required"] = list(curr_required) +# 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: +# { 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]]]] = { + "io.k8s.api.core.v1.HTTPGetAction": { + "port": ( + ("$ref", "#/definitions/io.k8s.apimachinery.pkg.util.intstr.IntOrString"), + ("type", "integer"), + ), + }, +} +for obj_name, field in INT_OR_STRING_FIELD_REMAPPING.items(): + try: + curr_field = spec["definitions"][obj_name] + except KeyError as e: + raise KeyError( + f"Could not find field {obj_name} in Argo specification for OpenAPI URI {open_api_spec_url}, " + f"caught error: {e}" + ) + + try: + properties = curr_field["properties"] + except KeyError as e: + raise KeyError( + f"Could not find properties for field {obj_name} in Argo specification for OpenAPI URI " + f"{open_api_spec_url}, caught error: {e}" + ) + + for property_to_change in field.keys(): + try: + curr_property = properties[property_to_change] + except KeyError as e: + raise KeyError( + f"Could not find property {property_to_change} for field {obj_name} in Argo specification for " + f"OpenAPI URI {open_api_spec_url}, caught error: {e}" + ) + + # 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] + + # check that the existing field and value are the same as the current field and value + assert curr_property[existing_field] == existing_value, ( + f"Expected to find field {existing_field} with value {existing_value} for property {property_to_change} " + f"for field {obj_name} in Argo specification for OpenAPI URI {open_api_spec_url}, but found " + f"{curr_property[existing_field]} instead" + ) + + # change the field and value + curr_property[new_field] = new_value + if existing_field != new_field: + del curr_property[existing_field] + # 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: diff --git a/src/hera/events/models/io/k8s/api/core/v1.py b/src/hera/events/models/io/k8s/api/core/v1.py index fdf10c608..3b00a2366 100644 --- a/src/hera/events/models/io/k8s/api/core/v1.py +++ b/src/hera/events/models/io/k8s/api/core/v1.py @@ -1526,7 +1526,7 @@ class HTTPGetAction(BaseModel): description=("Custom headers to set in the request. HTTP allows repeated headers."), ) path: Optional[str] = Field(default=None, description="Path to access on the HTTP server.") - port: intstr.IntOrString = Field( + port: int = Field( ..., description=( "Name or number of the port to access on the container. Number must be in" diff --git a/src/hera/events/models/io/k8s/api/core/v1.pyi b/src/hera/events/models/io/k8s/api/core/v1.pyi index e8b896464..2c2cd1099 100644 --- a/src/hera/events/models/io/k8s/api/core/v1.pyi +++ b/src/hera/events/models/io/k8s/api/core/v1.pyi @@ -369,7 +369,7 @@ class HTTPGetAction(BaseModel): host: Optional[str] http_headers: Optional[List[HTTPHeader]] path: Optional[str] - port: intstr.IntOrString + port: int scheme: Optional[Scheme] class ISCSIVolumeSource(BaseModel): diff --git a/src/hera/workflows/models/io/k8s/api/core/v1.py b/src/hera/workflows/models/io/k8s/api/core/v1.py index fdf10c608..3b00a2366 100644 --- a/src/hera/workflows/models/io/k8s/api/core/v1.py +++ b/src/hera/workflows/models/io/k8s/api/core/v1.py @@ -1526,7 +1526,7 @@ class HTTPGetAction(BaseModel): description=("Custom headers to set in the request. HTTP allows repeated headers."), ) path: Optional[str] = Field(default=None, description="Path to access on the HTTP server.") - port: intstr.IntOrString = Field( + port: int = Field( ..., description=( "Name or number of the port to access on the container. Number must be in" diff --git a/src/hera/workflows/models/io/k8s/api/core/v1.pyi b/src/hera/workflows/models/io/k8s/api/core/v1.pyi index e8b896464..2c2cd1099 100644 --- a/src/hera/workflows/models/io/k8s/api/core/v1.pyi +++ b/src/hera/workflows/models/io/k8s/api/core/v1.pyi @@ -369,7 +369,7 @@ class HTTPGetAction(BaseModel): host: Optional[str] http_headers: Optional[List[HTTPHeader]] path: Optional[str] - port: intstr.IntOrString + port: int scheme: Optional[Scheme] class ISCSIVolumeSource(BaseModel):