[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
clientless_app_ids=None (or []) should behave like omitting the key: no
getAppsByType call, and the PUT should be issued.
- 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.
[ISSUE] update_segment branches on the presence of
clientless_app_ids, and the Browser Access lookup it enters never matchesEnvironment
zscaler-sdk-python1.9.44 (editable install ofmaster@e7f5f7ef)application_segmentFound 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_segmentdecides whether to take the Browser Access code pathby testing for the presence of the
clientless_app_idskey rather than its value:Because
bodyiskwargs, any caller who passesclientless_app_ids=None— which is whathappens naturally when the argument set is assembled from a config file, a dataclass, or a
**payloadsplat where optional fields default toNone— is routed into that branch eventhough 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:
segments_listholdsAppSegmentByTypemodel instances. That model stores the field asapp_id(zscaler/zpa/models/application_segment.py:1048), andZscalerObject.getishasattr-based (zscaler/oneapi_object.py:25-38), soapp.get("appId")returnsNoneforevery element.
matched_segmentis thereforeNonefor any non-Nonesegment_id, and themethod returns:
Net effect: passing
clientless_app_idsin any form makesupdate_segmentfail — withNone, with[], and with a well-formed list on a segment that genuinely does have a BrowserAccess app.
Two smaller observations in the same block:
clientless_app_idswasNone, line 429(
for app in clientless_apps:) would raise an uncaughtTypeError.strin the third tuple slot;APIResult's third element isannotated
Optional[Exception](zscaler/types.py:39).Related, and three lines above:
server_group_idsuses the same presence-then-use shape(
zscaler/zpa/application_segment.py:408-409), whereserver_group_ids=NoneraisesTypeError: 'NoneType' object is not iterablerather than returning an error. Whateverconvention you settle on for "key present but value is
None" would ideally cover both.For contrast,
add_segmentdoes not do any of this — it simply assigns the value(
zscaler/zpa/application_segment.py:293-294), as doesadd_segment_provision(
:782-783). Onlyupdate_segmentperforms the lookup.Reproduction
Fully offline — no credentials, no network. Save as
repro.pyand run from a checkout.The equivalent live call is an ordinary update that happens to carry a
None:Expected behaviour
clientless_app_ids=None(or[]) should behave like omitting the key: nogetAppsByTypecall, and thePUTshould be issued.find it rather than reporting that none exists.
Actual behaviour
No
PUTis ever issued in any of the three cases — the update silently does not happen, andthe caller gets an error that talks about Browser Access even when they never mentioned it.
Root cause
zscaler/zpa/application_segment.py:411if "clientless_app_ids" in body:— presence, not valuezscaler/zpa/application_segment.py:421app.get("appId")onAppSegmentByTypeinstanceszscaler/zpa/models/application_segment.py:1048self.app_idzscaler/oneapi_object.py:25-38ZscalerObject.getishasattr-based, soget("appId")is alwaysNonezscaler/zpa/application_segment.py:423-424zscaler/zpa/application_segment.py:429for app in clientless_apps— would raise onNoneif the lookup did matchSuggested direction
I don't want to presume the intended design here, so listing what I can see rather than
proposing a patch:
if body.get("clientless_app_ids"):), orpop-and-test, or keep the presence check but treat
None/[]as "clear" and skip thelookup — 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.
app.app_id,app.get("app_id"), or keepingthe raw dicts instead of model instances would each work.
getAppsByTyperound trip could be dropped entirely and callersasked to supply the
clientlessAppsidthemselves, which would also remove an extra APIcall from every Browser Access update.
Happy to open a PR for whichever shape you'd prefer.