Skip to content

[ISSUE] The ZPA empty-body update placeholder returns model defaults (e.g. enabled=True) that a caller cannot tell apart from real values #582

Description

@hackerboey

[ISSUE] The ZPA empty-body update placeholder returns model defaults (e.g. enabled=True) that a caller cannot tell apart from real values

Environment

SDK version zscaler-sdk-python 1.9.44 (editable install of master @ e7f5f7ef)
Python 3.11.15
OS macOS (arm64)
Affected product / resource ZPA — 59 update methods across 42 modules (list below)

Found by reading the SDK source and confirmed with the offline reproduction below (a stub
request executor, so no credentials and no network are involved). All identifiers used are
synthetic.


This is deliberately narrower than #289 and #419 — please read this part first

#289 and #419 both asked why update_segment doesn't return the updated field values, and
both were answered: ZPA replies 204 No Content on updates, the SDK cannot populate a full
model, and callers should follow an update with a get_* call. That answer is clear and I'm
not relitigating it.

What neither issue covered, and what this one is about, is that the placeholder the SDK
substitutes is not empty — it carries the model's constructor defaults, and some of them
are affirmative. ApplicationSegments({"id": segment_id}) reports enabled=True, because
zscaler/zpa/models/application_segment.py:42 is:

self.enabled = config["enabled"] if "enabled" in config else True

So as_dict() on the placeholder emits {'id': ..., 'enabled': True, 'domain_names': [], 'tcp_port_ranges': [], 'udp_port_ranges': [], ...} — values the API never sent. A caller who
logs, diffs, caches, or asserts on the returned object is reading fabricated data, and since
err is None and the object is a real ApplicationSegments, nothing distinguishes it from a
response that genuinely said enabled=True with no domains. Both of the linked issues'
docstring-style usage (print(f"... {update_segment.as_dict()}")) hits exactly this.

I acknowledge a caller can disambiguate today via response.get_status()
(zscaler/oneapi_response.py:163) — that's a reasonable answer, and if it's the intended one
I'd be glad to send a docs-only PR saying so, since none of the 59 docstrings mention it.

Description

The pattern is:

# zscaler/zpa/application_segment.py:464-465
if response is None or not response.get_body():
    return (ApplicationSegments({"id": segment_id}), response, None)

It appears 59 times across 42 modules, all of them under zscaler/zpa/, and nowhere else
in the SDK:

$ python - <<'PY'
import re, glob, collections
guard = re.compile(r'if (?:response is None or not response\.get_body\(\)'
                   r'|not response or not response\.get_body\(\))\s*:')
ret = re.compile(r'return \((\w+)\(\{"id": ([a-z_]+)\}\), response, None\)')
total, files = 0, collections.OrderedDict()
for f in sorted(glob.glob('zscaler/**/*.py', recursive=True)):
    lines = open(f).read().split('\n')
    for i, line in enumerate(lines):
        if guard.search(line):
            for j in range(i + 1, min(i + 4, len(lines))):
                if ret.search(lines[j]):
                    total += 1
                    files.setdefault(f, []).append(i + 1)
                    break
print('placeholder returns:', total)
print('modules:', len(files))
print('by service:', dict(collections.Counter(f.split('/')[1] for f in files)))
PY
placeholder returns: 59
modules: 42
by service: {'zpa': 42}

Other services chose the opposite convention for the same situation, which is the part I find
hard to reconcile:

  • zscaler/zia/locations.py:565-570 — for a 204, returns (None, None) with the comment
    "This is the expected case for 204 No Content". No object is fabricated.
  • zscaler/zcell/sim_handling.py:230-231, zscaler/zcell/anomaly_policy.py:208-209,
    zscaler/zcell/customer_data_handling.py:125-126 — if not response or not response.get_body(): return (None, response, None). Same guard, honest return value.
  • ZIA / ZTW / ZCC / ZDX / ZIdentity update methods have no such guard at all — e.g.
    zscaler/zia/url_filtering.py update_rule parses the body directly.

So the "return a stub" choice is specific to ZPA, and within ZPA it returns defaults rather
than None. I'm raising the defaults part, not the "why isn't it populated" part.

Reproduction

Fully offline — no credentials, no network. The stub replies with an empty body, which is what
RequestExecutor.execute produces for a 204 (zscaler/request_executor.py:545-559
constructs a ZscalerAPIResponse with response_body={}).

from zscaler.zpa.application_segment import ApplicationSegmentAPI

SEGMENT_ID = "72058304855090128"
CONFIG = {"client": {"customerId": "72058304855015000"}}


class StubResponse:
    def get_body(self):
        return {}          # what a 204 yields

    def get_results(self):
        return []

    def get_status(self):
        return 204


class StubExecutor:
    """Stands in for RequestExecutor."""

    def create_request(self, method, endpoint, body=None, headers=None, params=None,
                       use_raw_data_for_body=False):
        return {"method": method, "endpoint": endpoint.strip(),
                "body": {} if body is None else body, "params": params or {}}, None

    def execute(self, request, response_type=None, return_raw_response=False):
        return StubResponse(), None


seg, resp, err = ApplicationSegmentAPI(StubExecutor(), CONFIG).update_segment(
    SEGMENT_ID, description="new description")

print("err            =", err)
print("type           =", type(seg).__name__)
print("seg.enabled    =", seg.enabled, "  <- never sent by the API")
print("seg.domain_names =", seg.domain_names, "  <- never sent by the API")
print("as_dict()      =", seg.as_dict())

Expected behaviour

The returned value should not assert field values the API did not send. Any of these would
achieve that; I'm not assuming which you'd want.

Actual behaviour

err            = None
type           = ApplicationSegments
seg.enabled    = True   <- never sent by the API
seg.domain_names = []   <- never sent by the API
as_dict()      = {'id': '72058304855090128', 'domain_names': [], 'server_groups': [], 'enabled': True, 'tcp_port_ranges': [], 'udp_port_ranges': [], 'tcp_port_range': [], 'udp_port_range': [], 'pra_apps': [], 'guest_details': []}

Root cause

Location Code
zscaler/zpa/application_segment.py:464-465 representative instance of the placeholder (58 more like it)
zscaler/zpa/models/application_segment.py:42 self.enabled = ... else True — the affirmative default that leaks into the placeholder
zscaler/zia/locations.py:565-570 the other convention in the same codebase, for the same 204
zscaler/zcell/sim_handling.py:230-231 ditto, (None, response, None)

Suggested direction

Deliberately not prescribing, especially as the surrounding behaviour has already been
declared intentional:

  • Leave the behaviour and document it — add a line to the ZPA update docstrings saying the
    returned object carries only the id, that other fields are model defaults and not API
    data, and that response.get_status() distinguishes the case. Smallest, non-breaking, and
    I'd be happy to write it.
  • Return (None, response, None) on an empty body, matching zscaler/zcell/* and
    zscaler/zia/locations.py. Honest, but breaking for anyone reading .id off the result.
  • Keep returning an object but construct it so unknown fields are None rather than
    constructor defaults, so as_dict() (which already drops None) yields just the id.

Which of these fits your plans? I'm happy to open the PR once you've picked.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions