Skip to content

fix: accept nullable external asset service status codes - #215

Open
Spplatten wants to merge 1 commit into
CrowdStrike:mainfrom
Spplatten:fix/nullable-external-asset-status
Open

Spplatten wants to merge 1 commit into
CrowdStrike:mainfrom
Spplatten:fix/nullable-external-asset-status

Conversation

@Spplatten

Copy link
Copy Markdown

Description

get_external_assets rejects an entire response when an IP or DNS service has
status_code: null, as reported in #214. Represent the service status code as
Option<i32> so unknown values remain distinct from numeric zero.

Fixes #214.

Changes

  • Deserialize null or omitted status codes as None, and integers as Some(code).
  • Keep the serialized status_code key: None serializes as JSON null.
  • Preserve the existing new(..., status_code: i32, ...) constructor signature by
    wrapping its argument in Some.
  • Keep invalid types and out-of-range integers as deserialization errors.
  • Leave port unchanged: the report establishes null status codes, not null ports.
  • Add six model tests and two loopback HTTP tests using the real
    get_external_assets method for both IP and DNS assets.
  • Document the public-field type change in the changelog.

Checklist

  • Examples work/pass (live examples require Falcon credentials)
  • No sensitive information has been committed
  • Changelog file has been updated
  • Code generates no warnings (Clippy and formatting checks pass)
  • "Noisy" (WIP) commits have been squashed for better commit hygiene and cleaner history

Additional Notes

Compatibility: the public field changes from i32 to Option<i32>, so direct
field reads, writes, and struct literals may need updates. The constructor's
integer argument remains source-compatible. The derived default status changes
from zero to None.

Generated source: the model is generated, but the source OpenAPI specification
is not tracked in this repository (swagger** and /specs are ignored). Please
carry the nullable status-code correction into the maintained specification before
regeneration. This PR does not claim to update that unavailable specification.

Evidence: all fixtures are synthetic. The HTTP tests bind only to a local mock
server and do not use Falcon credentials. Live API behavior has not been independently
validated; the null response shape comes from the issue report.

Validation on Windows with Rust 1.98.0:

  • cargo test --locked --all-features --all-targets: 18 passed, zero failed,
    including all eight new regression tests. Example targets compile; live examples
    and the credential-dependent doctest were not executed.
  • Negative control on upstream 6106cef85514d8b849fc29cfce3ff5e0dc00b197:
    four of the eight new tests fail (null, missing field, IP response, DNS response);
    the four existing-behavior controls pass. Both HTTP failures reproduce
    invalid type: null, expected i32 using the real SDK method.
  • cargo clippy --locked --all-features --test external_asset_status_code --test exposure_management_api_test -- '-Dwarnings' '-Aclippy::single-match-else' '-Adeprecated': passed.
  • cargo fmt --check and staged/unstaged git diff --check: passed.

The local Windows build used CMake/Ninja, AWS-LC's portable C path
(AWS_LC_SYS_NO_ASM=1), embedded MSVC debug information (_CL_=/Z7), and
CARGO_PROFILE_DEV_DEBUG=0 to work around native build failures in the execution
environment. Cargo's cache and build output were placed under the workspace's
ignored target directory. No dependency or TLS feature changes are included
in the contribution.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

get_external_assets fails to deserialize valid Exposure Management assets when nullable numeric fields are returned

1 participant