Skip to content

Commit 8073015

Browse files
wmudgersuplina
andauthored
Update API client parameters for legacy alignment and CDP control plane region (#215)
* Add API parameters for endpoint_region, endpoint_tls. * Add CDP_REGION environment variable for endpoint_region. * Add legacy parameter aliases. * Read CDP control plane region from configuration. * Align TLS validation with underlying URL request parameters * Update pagination to handle multiple "next token" parameters Signed-off-by: Webster Mudge <wmudge@cloudera.com> Signed-off-by: rsuplina <rsuplina@cloudera.com> Co-authored-by: rsuplina <rsuplina@cloudera.com>
1 parent 90fbd58 commit 8073015

File tree

6 files changed

+429
-26
lines changed

6 files changed

+429
-26
lines changed

plugins/doc_fragments/cdp_client.py

Lines changed: 44 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,20 +50,63 @@ class ModuleDocFragment(object):
5050
endpoint:
5151
description:
5252
- The Cloudera on cloud API endpoint to use.
53+
- Mutually exclusive with O(endpoint_region).
5354
type: str
54-
required: True
55+
required: False
5556
aliases:
57+
- endpoint_url
5658
- url
59+
endpoint_region:
60+
description:
61+
- Specify the Cloudera on cloud API endpoint region.
62+
- See L(Cloudera Control Plane regions,https://docs.cloudera.com/cdp-public-cloud/cloud/cp-regions/topics/cdp-control-plane-regions.html) for more information.
63+
- If not provided, the API will attempt to use the value from the environment variable E(CDP_REGION).
64+
- V(default) is an alias for the V(us-west-1) region.
65+
- Mutually exclusive with O(endpoint).
66+
type: str
67+
required: False
68+
default: "us-west-1"
69+
choices:
70+
- default
71+
- us-west-1
72+
- eu-1
73+
- ap-1
74+
aliases:
75+
- cdp_endpoint_region
76+
- cdp_region
77+
- region
78+
endpoint_tls:
79+
description:
80+
- Verify the TLS certificates for the Cloudera on cloud API endpoint.
81+
type: bool
82+
required: False
83+
default: True
84+
aliases:
85+
- verify_endpoint_tls
86+
- verify_tls
87+
- verify_api_tls
5788
debug:
5889
description:
5990
- If C(true), the module will capture the Cloudera on cloud HTTP log and return it in the RV(sdk_out) and RV(sdk_out_lines) fields.
6091
type: bool
6192
required: False
6293
default: False
94+
aliases:
95+
- debug_endpoints
6396
http_agent:
6497
description:
6598
- The HTTP user agent to use for Cloudera on cloud API requests.
6699
type: str
67100
required: False
68101
default: "cloudera.cloud"
102+
aliases:
103+
- agent_header
104+
strict:
105+
description:
106+
- Legacy CDPy SDK error handling.
107+
type: bool
108+
required: False
109+
default: False
110+
aliases:
111+
- strict_errors
69112
"""

plugins/module_utils/cdp_client.py

Lines changed: 31 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -44,16 +44,17 @@ class CdpCredentialError(Exception):
4444
def load_cdp_config(
4545
credentials_path: str,
4646
profile: str,
47-
) -> Tuple[str, str]:
47+
) -> Tuple[str, str, str]:
4848
"""
49-
Load CDP credential configuration by parsing credential file.
49+
Load CDP credential configuration by parsing credential file. If the profile has a region specified, it will be
50+
returned; otherwise, the default region "us-west-1" will be used.
5051
5152
Args:
5253
credentials_path: Path to CDP credentials file (supports ~ expansion)
5354
profile: Profile name to load from the credentials file
5455
5556
Returns:
56-
Tuple of (access_key, private_key)
57+
Tuple of (access_key, private_key, region)
5758
5859
Raises:
5960
CdpCredentialError: If file doesn't exist, profile not found, or keys missing
@@ -85,7 +86,13 @@ def load_cdp_config(
8586
msg = "CDP profile '{0}' is missing 'cdp_private_key'"
8687
raise CdpCredentialError(msg.format(profile))
8788

88-
return access_key, private_key
89+
# Load region
90+
if config.has_option(profile, "cdp_region"):
91+
region = config.get(profile, "cdp_region")
92+
else:
93+
region = "us-west-1"
94+
95+
return access_key, private_key, region
8996

9097

9198
def create_canonical_request_string(
@@ -305,7 +312,17 @@ def wrapper(self, *args, **kwargs):
305312
# Get the initial response
306313
response = func(self, *args, **paginated_kwargs)
307314

308-
if not isinstance(response, dict) or "nextPageToken" not in response:
315+
if not isinstance(response, dict):
316+
return response
317+
318+
# Determine which pagination token is used
319+
next_token_key = None
320+
if "nextPageToken" in response:
321+
next_token_key = "nextPageToken"
322+
elif "nextToken" in response:
323+
next_token_key = "nextToken"
324+
else:
325+
# No pagination token found, return as-is
309326
return response
310327

311328
# Collect all items from paginated responses
@@ -320,9 +337,9 @@ def wrapper(self, *args, **kwargs):
320337
else:
321338
all_items[key] = value
322339

323-
# Continue pagination while nextPageToken exists
324-
while "nextPageToken" in all_items:
325-
token = all_items.pop("nextPageToken")
340+
# Continue pagination while nextToken exists
341+
while next_token_key in all_items:
342+
token = all_items.pop(next_token_key)
326343

327344
# Add pagination parameters
328345
paginated_kwargs = kwargs.copy()
@@ -492,6 +509,12 @@ def _make_request(
492509
self.private_key,
493510
)
494511

512+
# Populate validate_certs from endpoint_tls
513+
self.module.params["validate_certs"] = self.module.params.get(
514+
"endpoint_tls",
515+
True,
516+
)
517+
495518
# Add query parameters to URL if provided
496519
if params:
497520
# Handle list parameters (e.g., guid=[guid1, guid2])

plugins/module_utils/common.py

Lines changed: 47 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -135,19 +135,46 @@ def __init__(
135135
fallback=(env_fallback, ["CDP_PROFILE"]),
136136
default="default",
137137
),
138-
endpoint=dict(required=True, type="str", aliases=["url"]),
139-
debug=dict(required=False, type="bool", default=False),
138+
endpoint=dict(
139+
required=False,
140+
type="str",
141+
aliases=["endpoint_url", "url"],
142+
),
143+
endpoint_region=dict(
144+
required=False,
145+
type="str",
146+
fallback=(env_fallback, ["CDP_REGION"]),
147+
# default="us-west-1", # NOTE: Handled by load_cdp_region()
148+
aliases=["cdp_endpoint_region", "cdp_region", "region"],
149+
choices=["default", "us-west-1", "eu-1", "ap-1"],
150+
),
151+
endpoint_tls=dict(
152+
required=False,
153+
type="bool",
154+
default=True,
155+
aliases=["verify_endpoint_tls", "verify_tls", "verify_api_tls"],
156+
),
157+
debug=dict(
158+
required=False,
159+
type="bool",
160+
default=False,
161+
aliases=["debug_endpoints"],
162+
),
140163
http_agent=dict(
141164
required=False,
142165
type="str",
143166
default="cloudera.cloud",
167+
aliases=["agent_header"],
144168
),
145169
),
146170
required_together=required_together + [["access_key", "private_key"]],
147171
bypass_checks=bypass_checks,
148172
no_log=no_log,
149173
mutually_exclusive=mutually_exclusive
150-
+ [["access_key", "credentials_path"]],
174+
+ [
175+
["access_key", "credentials_path"],
176+
["endpoint", "endpoint_region"],
177+
],
151178
required_one_of=required_one_of,
152179
add_file_common_args=add_file_common_args,
153180
supports_check_mode=supports_check_mode,
@@ -162,10 +189,11 @@ def __init__(
162189
# Load CDP credentials - check if provided via parameters first
163190
access_key = self.get_param("access_key")
164191
private_key = self.get_param("private_key")
192+
region = self.get_param("endpoint_region")
165193

166-
# If either credential is missing, load from credentials file
167-
if access_key is None or private_key is None:
168-
file_access_key, file_private_key = load_cdp_config(
194+
# If any credential is missing, load from credentials file
195+
if access_key is None or private_key is None or region is None:
196+
file_access_key, file_private_key, file_region = load_cdp_config(
169197
credentials_path=self.get_param("credentials_path"),
170198
profile=self.get_param("profile"),
171199
)
@@ -174,10 +202,23 @@ def __init__(
174202
access_key = file_access_key
175203
if private_key is None:
176204
private_key = file_private_key
205+
if region is None:
206+
region = file_region
177207

178208
self.access_key: str = access_key
179209
self.private_key: str = private_key
180210

211+
# Handle legacy parameter value
212+
if region == "default":
213+
self.endpoint_region = "us-west-1"
214+
else:
215+
self.endpoint_region: str = region
216+
217+
# NOTE: If endpoint is not provided, construct the endpoint parameter from the region
218+
# NOTE: IAM endpoints for us-west-1 need to be explicitly set to iamapi.us-west-1.altus.cloudera.com
219+
if self.endpoint is None:
220+
self.endpoint = f"https://api.{self.endpoint_region}.cdp.cloudera.com"
221+
181222
# Initialize mixins parameters
182223
for base in self.__class__.__mro__:
183224
if (

tests/unit/conftest.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -133,9 +133,9 @@ def mock_ansible_module(mocker):
133133
@pytest.fixture()
134134
def mock_load_cdp_config(mocker):
135135
"""Mock the load_cdp_config function."""
136-
mocker.patch(
136+
return mocker.patch(
137137
"ansible_collections.cloudera.cloud.plugins.module_utils.common.load_cdp_config",
138-
return_value=("test-access-key", "test-private-key"),
138+
return_value=("test-access-key", "test-private-key", "test-region"),
139139
)
140140

141141

@@ -146,6 +146,7 @@ def unset_cdp_env_vars(monkeypatch):
146146
monkeypatch.delenv("CDP_PRIVATE_KEY", raising=False)
147147
monkeypatch.delenv("CDP_CREDENTIALS_PATH", raising=False)
148148
monkeypatch.delenv("CDP_PROFILE", raising=False)
149+
monkeypatch.delenv("CDP_REGION", raising=False)
149150

150151

151152
@pytest.fixture()

tests/unit/plugins/module_utils/cdp_client/test_load_cdp_config.py

Lines changed: 30 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,13 +38,37 @@ def test_load_cdp_config_reads_from_file_successfully(mocker):
3838
mock_open_object = mocker.mock_open(read_data=config_content)
3939
mocker.patch("builtins.open", mock_open_object)
4040

41-
result_access, result_private = load_cdp_config(
41+
result_access, result_private, result_region = load_cdp_config(
4242
credentials_path="/mock/path",
4343
profile="default",
4444
)
4545

4646
assert result_access == "file-access-key"
4747
assert result_private == "file-private-key"
48+
assert result_region == "us-west-1"
49+
50+
51+
def test_load_cdp_config_reads_from_file_successfully_region(mocker):
52+
"""Test successful reading of credentials from configuration file with cdp_region."""
53+
54+
config_content = """[default]
55+
cdp_access_key_id = file-access-key
56+
cdp_private_key = file-private-key
57+
cdp_region = file-region
58+
"""
59+
60+
mocker.patch("os.path.exists", return_value=True)
61+
mock_open_object = mocker.mock_open(read_data=config_content)
62+
mocker.patch("builtins.open", mock_open_object)
63+
64+
result_access, result_private, result_region = load_cdp_config(
65+
credentials_path="/mock/path",
66+
profile="default",
67+
)
68+
69+
assert result_access == "file-access-key"
70+
assert result_private == "file-private-key"
71+
assert result_region == "file-region"
4872

4973

5074
def test_load_cdp_config_missing_credentials_file(mocker):
@@ -124,19 +148,21 @@ def test_load_cdp_config_custom_profile(mocker):
124148
[production]
125149
cdp_access_key_id = prod-access-key
126150
cdp_private_key = prod-private-key
151+
cdp_region = prod-region
127152
"""
128153

129154
mocker.patch("os.path.exists", return_value=True)
130155
mock_open_object = mocker.mock_open(read_data=config_content)
131156
mocker.patch("builtins.open", mock_open_object)
132157

133-
result_access, result_private = load_cdp_config(
158+
result_access, result_private, result_region = load_cdp_config(
134159
credentials_path="/mock/path",
135160
profile="production",
136161
)
137162

138163
assert result_access == "prod-access-key"
139164
assert result_private == "prod-private-key"
165+
assert result_region == "prod-region"
140166

141167

142168
def test_load_cdp_config_expands_user_path(mocker):
@@ -160,7 +186,7 @@ def test_load_cdp_config_expands_user_path(mocker):
160186
mock_open_object = mocker.mock_open(read_data=config_content)
161187
mocker.patch("builtins.open", mock_open_object)
162188

163-
result_access, result_private = load_cdp_config(
189+
result_access, result_private, result_region = load_cdp_config(
164190
credentials_path="~/.cdp/credentials",
165191
profile="default",
166192
)
@@ -172,6 +198,7 @@ def test_load_cdp_config_expands_user_path(mocker):
172198
# Verify credentials were loaded
173199
assert result_access == "test-access-key"
174200
assert result_private == "test-private-key"
201+
assert result_region == "us-west-1"
175202

176203

177204
def test_load_cdp_config_path_expansion_error_message(mocker):

0 commit comments

Comments
 (0)