Skip to content

infrahubctl check to find and filter the same as PC checks #446

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: stable
Choose a base branch
from
Open

infrahubctl check to find and filter the same as PC checks #446

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b264b04
Add initial fix to 445, but need additional feedback
FragmentedPacket Jun 14, 2025
dd1d2a2
Making the filter for the targets to be statically set as this should…
FragmentedPacket Jun 14, 2025
e973fc1
Fix filter to not use kwargs unpacking.
FragmentedPacket Jun 14, 2025
1159059
Had to change filters to unpack kwargs due to MyPy typing issue.
FragmentedPacket Jun 14, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 8 additions & 10 deletions infrahub_sdk/ctl/check.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,15 +137,13 @@
variables: dict[str, str],
branch: str | None = None,
) -> bool:
filters = {}
param_value = list(check_module.definition.parameters.values())
if param_value:
filters[param_value[0]] = check_module.definition.targets

param_key = list(check_module.definition.parameters.keys())
filters = {"name__value": check_module.definition.targets}

Check warning on line 140 in infrahub_sdk/ctl/check.py

View check run for this annotation

Codecov / codecov/patch

infrahub_sdk/ctl/check.py#L140 

Added line #L140 was not covered by tests
identifier = None
if param_key:
identifier = param_key[0]
identifier_attribute = None

Check warning on line 142 in infrahub_sdk/ctl/check.py

View check run for this annotation

Codecov / codecov/patch

infrahub_sdk/ctl/check.py#L142 

Added line #L142 was not covered by tests
# TODO: Does this support multiple parameters and we need make identifiers a dict to iterate over on L166-168?
for param_key, param_value in check_module.definition.parameters.items():
identifier = param_key
identifier_attribute = param_value.split("__")[0]

Check warning on line 146 in infrahub_sdk/ctl/check.py

View check run for this annotation

Codecov / codecov/patch

infrahub_sdk/ctl/check.py#L145-L146 

Added lines #L145 - L146 were not covered by tests
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this might be fine as it resolves a previous error. But I'm not sure these params really work today if you have more than one. This is something that we should address. It's also problematic with regards to a query that might look like this instead:

query TagsQuery($tag_name: String!) {
  BuiltinTag(name__value: $tag_name) {
    edges {
      node {
        description {
          value
        }
      }
    }
  }
}

I.e. it's a valid check but the name is missing and as such will be reported as null.

Have you tried to run this same check and code within the pipeline in Infrahub as well? I think we might need to fix something there too.


check_summary: list[bool] = []
if variables:
Expand All @@ -164,8 +162,8 @@
await targets.members.fetch()
for member in targets.members.peers:
check_parameter = {}
if identifier:
attribute = getattr(member.peer, identifier)
if identifier and identifier_attribute:
attribute = getattr(member.peer, identifier_attribute)

Check warning on line 166 in infrahub_sdk/ctl/check.py

View check run for this annotation

Codecov / codecov/patch

infrahub_sdk/ctl/check.py#L166 

Added line #L166 was not covered by tests
check_parameter = {identifier: attribute.value}
result = await run_check(
check_module=check_module,
Expand Down