Skip to content

Commit

Permalink
Add support for modifying field properties prior to model generation (#…
Browse files Browse the repository at this point in the history
…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 <[email protected]>
  • Loading branch information
flaviuvadan authored Nov 23, 2023
1 parent 862c45b commit 6cba295
Show file tree
Hide file tree
Showing 7 changed files with 62 additions and 7 deletions.
2 changes: 1 addition & 1 deletion docs/examples/workflows/upstream/daemon_nginx.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion examples/workflows/upstream/daemon-nginx.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ spec:
readinessProbe:
httpGet:
path: /
port: '80'
port: 80
initialDelaySeconds: 2
timeoutSeconds: 1
daemon: true
Expand Down
57 changes: 56 additions & 1 deletion scripts/spec.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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:
Expand Down
2 changes: 1 addition & 1 deletion src/hera/events/models/io/k8s/api/core/v1.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
2 changes: 1 addition & 1 deletion src/hera/events/models/io/k8s/api/core/v1.pyi
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
2 changes: 1 addition & 1 deletion src/hera/workflows/models/io/k8s/api/core/v1.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
2 changes: 1 addition & 1 deletion src/hera/workflows/models/io/k8s/api/core/v1.pyi
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down

0 comments on commit 6cba295

Please sign in to comment.