Skip to content

[ISSUE] update_segment branches on the presence of clientless_app_ids, and the Browser Access lookup it enters never matches #581

Description

@hackerboey

[ISSUE] update_segment branches on the presence of clientless_app_ids, and the Browser Access lookup it enters never matches

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 / application_segment

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.


Description

ApplicationSegmentAPI.update_segment decides whether to take the Browser Access code path
by testing for the presence of the clientless_app_ids key rather than its value:

# zscaler/zpa/application_segment.py:411
if "clientless_app_ids" in body:
    clientless_apps = body.pop("clientless_app_ids")
    ...

Because body is kwargs, any caller who passes clientless_app_ids=None — which is what
happens naturally when the argument set is assembled from a config file, a dataclass, or a
**payload splat where optional fields default to None — is routed into that branch even
though they never asked to touch Browser Access. clientless_app_ids=[] is routed there too.

Once inside the branch there is a second problem. The branch looks up the segment's existing
Browser Access app with:

# zscaler/zpa/application_segment.py:421
matched_segment = next((app for app in segments_list if app.get("appId") == segment_id), None)

segments_list holds AppSegmentByType model instances. That model stores the field as
app_id (zscaler/zpa/models/application_segment.py:1048), and ZscalerObject.get is
hasattr-based (zscaler/oneapi_object.py:25-38), so app.get("appId") returns None for
every element. matched_segment is therefore None for any non-None segment_id, and the
method returns:

Error: No matching clientless App found with appId '<segment_id>' in existing segments.

Net effect: passing clientless_app_ids in any form makes update_segment fail — with
None, with [], and with a well-formed list on a segment that genuinely does have a Browser
Access app.

Two smaller observations in the same block:

  • If the lookup ever did match while clientless_app_ids was None, line 429
    (for app in clientless_apps:) would raise an uncaught TypeError.
  • The failure is returned as a str in the third tuple slot; APIResult's third element is
    annotated Optional[Exception] (zscaler/types.py:39).

Related, and three lines above: server_group_ids uses the same presence-then-use shape
(zscaler/zpa/application_segment.py:408-409), where server_group_ids=None raises
TypeError: 'NoneType' object is not iterable rather than returning an error. Whatever
convention you settle on for "key present but value is None" would ideally cover both.

For contrast, add_segment does not do any of this — it simply assigns the value
(zscaler/zpa/application_segment.py:293-294), as does add_segment_provision
(:782-783). Only update_segment performs the lookup.

Reproduction

Fully offline — no credentials, no network. Save as repro.py and run from a checkout.

from zscaler.zpa.application_segment import ApplicationSegmentAPI
from zscaler.zpa.models.application_segment import AppSegmentByType

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


class StubResponse:
    def __init__(self, body):
        self._body = body

    def get_body(self):
        return self._body

    def get_results(self):
        return self._body if isinstance(self._body, list) else []

    def get_status(self):
        return 200


class StubExecutor:
    """Stands in for RequestExecutor: records requests, replies with canned bodies."""

    def __init__(self, by_type_payload=None):
        self.requests = []
        self.by_type_payload = by_type_payload or []

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

    def execute(self, request, response_type=None, return_raw_response=False):
        if "getAppsByType" in request["endpoint"]:
            return StubResponse(self.by_type_payload), None
        return StubResponse({}), None


# 1. clientless_app_ids=None, segment has no Browser Access app
ex = StubExecutor(by_type_payload=[])
seg, resp, err = ApplicationSegmentAPI(ex, CONFIG).update_segment(
    SEGMENT_ID, name="segment-a", clientless_app_ids=None)
print("1. err =", repr(err))
print("   requests:", [(r["method"], r["endpoint"].split("/")[-1]) for r in ex.requests])

# 2. clientless_app_ids=[]
ex = StubExecutor(by_type_payload=[])
seg, resp, err = ApplicationSegmentAPI(ex, CONFIG).update_segment(
    SEGMENT_ID, name="segment-a", clientless_app_ids=[])
print("2. err =", repr(err))

# 3. a well-formed list, with a MATCHING Browser Access app present
print("3. AppSegmentByType(...).get('appId') =",
      AppSegmentByType({"id": "72058304855099001", "appId": SEGMENT_ID}).get("appId"))
print("   AppSegmentByType(...).app_id       =",
      AppSegmentByType({"id": "72058304855099001", "appId": SEGMENT_ID}).app_id)
ex = StubExecutor(by_type_payload=[
    {"id": "72058304855099001", "appId": SEGMENT_ID, "name": "app.example.com"}])
seg, resp, err = ApplicationSegmentAPI(ex, CONFIG).update_segment(
    SEGMENT_ID, name="segment-a",
    clientless_app_ids=[{"name": "app.example.com", "enabled": True,
                         "domain": "app.example.com"}])
print("   err =", repr(err))

The equivalent live call is an ordinary update that happens to carry a None:

client.zpa.application_segment.update_segment(
    segment_id="72058304855090128",
    name="segment-a",
    clientless_app_ids=None,      # optional field that was left unset upstream
)

Expected behaviour

  1. clientless_app_ids=None (or []) should behave like omitting the key: no
    getAppsByType call, and the PUT should be issued.
  2. When a caller does pass a list and the segment has a Browser Access app, the lookup should
    find it rather than reporting that none exists.

Actual behaviour

1. err = "Error: No matching clientless App found with appId '72058304855090128' in existing segments."
   requests: [('GET', 'getAppsByType')]
2. err = "Error: No matching clientless App found with appId '72058304855090128' in existing segments."
3. AppSegmentByType(...).get('appId') = None
   AppSegmentByType(...).app_id       = 72058304855090128
   err = "Error: No matching clientless App found with appId '72058304855090128' in existing segments."

No PUT is ever issued in any of the three cases — the update silently does not happen, and
the caller gets an error that talks about Browser Access even when they never mentioned it.

Root cause

Location Code
zscaler/zpa/application_segment.py:411 if "clientless_app_ids" in body: — presence, not value
zscaler/zpa/application_segment.py:421 app.get("appId") on AppSegmentByType instances
zscaler/zpa/models/application_segment.py:1048 the model stores it as self.app_id
zscaler/oneapi_object.py:25-38 ZscalerObject.get is hasattr-based, so get("appId") is always None
zscaler/zpa/application_segment.py:423-424 the resulting error return
zscaler/zpa/application_segment.py:429 for app in clientless_apps — would raise on None if the lookup did match

Suggested direction

I don't want to presume the intended design here, so listing what I can see rather than
proposing a patch:

  • The routing condition could gate on the value (if body.get("clientless_app_ids"):), or
    pop-and-test, or keep the presence check but treat None/[] as "clear" and skip the
    lookup — these differ in whether clientless_app_ids=[] should mean "leave alone" or
    "remove all Browser Access apps", which is a product decision I can't make.
  • The lookup key needs to agree with the model. app.app_id, app.get("app_id"), or keeping
    the raw dicts instead of model instances would each work.
  • Alternatively the implicit getAppsByType round trip could be dropped entirely and callers
    asked to supply the clientlessApps id themselves, which would also remove an extra API
    call from every Browser Access update.

Happy to open a PR for whichever shape you'd prefer.

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