Skip to content

Commit 74f874c

Browse files
cloudsmith-iduffycloudsmith-iduffy
committed
fix: review feedback
1 parent b4b2583 commit 74f874c

17 files changed

+154
-185
lines changed

‎cloudsmith_cli/cli/decorators.py‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,10 @@
77
from cloudsmith_cli.cli import validators
88

99
from ..core.api.init import initialise_api as _initialise_api
10-
from ..core.credentials import CredentialContext, CredentialProviderChain
11-
from ..core.credentials.session import create_session as _create_session
10+
from ..core.credentials.chain import CredentialProviderChain
11+
from ..core.credentials.models import CredentialContext
1212
from ..core.mcp import server
13+
from ..core.rest import create_requests_session as _create_session
1314
from . import config, utils
1415

1516

‎cloudsmith_cli/cli/tests/conftest.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55

66
from ...core.api.init import initialise_api
77
from ...core.api.repos import create_repo, delete_repo
8-
from ...core.credentials import CredentialResult
8+
from ...core.credentials.models import CredentialResult
99
from .utils import random_str
1010

1111

‎cloudsmith_cli/cli/webserver.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99

1010
from ..core.api.exceptions import ApiException
1111
from ..core.api.init import initialise_api
12-
from ..core.credentials import CredentialResult
12+
from ..core.credentials.models import CredentialResult
1313
from ..core.keyring import store_sso_tokens
1414
from .saml import exchange_2fa_token
1515

‎cloudsmith_cli/core/credentials/__init__.py‎

Lines changed: 0 additions & 100 deletions
Original file line numberDiff line numberDiff line change
@@ -1,100 +0,0 @@
1-
"""Credential Provider Chain for Cloudsmith CLI.
2-
3-
Implements an AWS SDK-style credential resolution chain that evaluates
4-
credential sources sequentially and returns the first valid result.
5-
"""
6-
7-
from __future__ import annotations
8-
9-
import logging
10-
from abc import ABC, abstractmethod
11-
from dataclasses import dataclass
12-
from typing import TYPE_CHECKING
13-
14-
if TYPE_CHECKING:
15-
import requests
16-
17-
logger = logging.getLogger(__name__)
18-
19-
20-
@dataclass
21-
class CredentialContext:
22-
"""Context passed to credential providers during resolution.
23-
24-
All values are populated directly from Click options / ``opts``.
25-
"""
26-
27-
session: requests.Session | None = None
28-
api_key: str | None = None
29-
api_host: str = "https://api.cloudsmith.io"
30-
creds_file_path: str | None = None
31-
profile: str | None = None
32-
debug: bool = False
33-
keyring_refresh_failed: bool = False
34-
35-
36-
@dataclass
37-
class CredentialResult:
38-
"""Result from a successful credential resolution."""
39-
40-
api_key: str
41-
source_name: str
42-
source_detail: str | None = None
43-
auth_type: str = "api_key"
44-
45-
46-
class CredentialProvider(ABC):
47-
"""Base class for credential providers."""
48-
49-
name: str = "base"
50-
51-
@abstractmethod
52-
def resolve(self, context: CredentialContext) -> CredentialResult | None:
53-
"""Attempt to resolve credentials. Return CredentialResult or None."""
54-
55-
56-
class CredentialProviderChain:
57-
"""Evaluates credential providers in order, returning the first valid result.
58-
59-
If no providers are given, uses the default chain:
60-
Keyring → CLIFlag.
61-
"""
62-
63-
def __init__(self, providers: list[CredentialProvider] | None = None):
64-
if providers is not None:
65-
self.providers = providers
66-
else:
67-
from .providers import CLIFlagProvider, KeyringProvider
68-
69-
self.providers = [
70-
KeyringProvider(),
71-
CLIFlagProvider(),
72-
]
73-
74-
def resolve(self, context: CredentialContext) -> CredentialResult | None:
75-
"""Evaluate each provider in order. Return the first successful result."""
76-
for provider in self.providers:
77-
try:
78-
result = provider.resolve(context)
79-
if result is not None:
80-
if context.debug:
81-
logger.debug(
82-
"Credentials resolved by %s: %s",
83-
provider.name,
84-
result.source_detail or result.source_name,
85-
)
86-
return result
87-
if context.debug:
88-
logger.debug(
89-
"Provider %s did not resolve credentials, trying next",
90-
provider.name,
91-
)
92-
except Exception: # pylint: disable=broad-exception-caught
93-
# Intentionally broad - one provider failing shouldn't stop others
94-
logger.debug(
95-
"Provider %s raised an exception, skipping",
96-
provider.name,
97-
exc_info=True,
98-
)
99-
continue
100-
return None

‎cloudsmith_cli/core/credentials/chain.py‎

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
"""Credential provider chain for the Cloudsmith CLI.
2+
3+
Implements an AWS SDK-style credential resolution chain that evaluates
4+
credential sources sequentially and returns the first valid result.
5+
"""
6+
7+
from __future__ import annotations
8+
9+
import logging
10+
11+
from .models import CredentialContext, CredentialResult
12+
from .provider import CredentialProvider
13+
14+
logger = logging.getLogger(__name__)
15+
16+
17+
class CredentialProviderChain:
18+
"""Evaluates credential providers in order, returning the first valid result.
19+
20+
If no providers are given, uses the default chain:
21+
Keyring → CLIFlag.
22+
"""
23+
24+
def __init__(self, providers: list[CredentialProvider] | None = None):
25+
if providers is not None:
26+
self.providers = providers
27+
else:
28+
from .providers import CLIFlagProvider, KeyringProvider
29+
30+
self.providers = [
31+
KeyringProvider(),
32+
CLIFlagProvider(),
33+
]
34+
35+
def resolve(self, context: CredentialContext) -> CredentialResult | None:
36+
"""Evaluate each provider in order. Return the first successful result."""
37+
for provider in self.providers:
38+
try:
39+
result = provider.resolve(context)
40+
if result is not None:
41+
if context.debug:
42+
logger.debug(
43+
"Credentials resolved by %s: %s",
44+
provider.name,
45+
result.source_detail or result.source_name,
46+
)
47+
return result
48+
if context.debug:
49+
logger.debug(
50+
"Provider %s did not resolve credentials, trying next",
51+
provider.name,
52+
)
53+
except Exception: # pylint: disable=broad-exception-caught
54+
# Intentionally broad - one provider failing shouldn't stop others
55+
logger.debug(
56+
"Provider %s raised an exception, skipping",
57+
provider.name,
58+
exc_info=True,
59+
)
60+
continue
61+
return None

‎cloudsmith_cli/core/credentials/models.py‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
"""Credential data models for the Cloudsmith CLI."""
2+
3+
from __future__ import annotations
4+
5+
from dataclasses import dataclass
6+
7+
import requests
8+
9+
10+
@dataclass
11+
class CredentialContext:
12+
"""Context passed to credential providers during resolution.
13+
14+
All values are populated directly from Click options / ``opts``.
15+
"""
16+
17+
session: requests.Session | None = None
18+
api_key: str | None = None
19+
api_host: str = "https://api.cloudsmith.io"
20+
creds_file_path: str | None = None
21+
profile: str | None = None
22+
debug: bool = False
23+
keyring_refresh_failed: bool = False
24+
25+
26+
@dataclass
27+
class CredentialResult:
28+
"""Result from a successful credential resolution."""
29+
30+
api_key: str
31+
source_name: str
32+
source_detail: str | None = None
33+
auth_type: str = "api_key"

‎cloudsmith_cli/core/credentials/provider.py‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
"""Base credential provider interface."""
2+
3+
from __future__ import annotations
4+
5+
from abc import ABC, abstractmethod
6+
7+
from .models import CredentialContext, CredentialResult
8+
9+
10+
class CredentialProvider(ABC):
11+
"""Base class for credential providers."""
12+
13+
name: str = "base"
14+
15+
@abstractmethod
16+
def resolve(self, context: CredentialContext) -> CredentialResult | None:
17+
"""Attempt to resolve credentials. Return CredentialResult or None."""

‎cloudsmith_cli/core/credentials/providers/cli_flag.py‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,8 @@
22

33
from __future__ import annotations
44

5-
from .. import CredentialContext, CredentialProvider, CredentialResult
5+
from ..models import CredentialContext, CredentialResult
6+
from ..provider import CredentialProvider
67

78

89
class CLIFlagProvider(CredentialProvider):

‎cloudsmith_cli/core/credentials/providers/keyring_provider.py‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,10 @@
44

55
import logging
66

7-
from .. import CredentialContext, CredentialProvider, CredentialResult
7+
from ....cli.saml import refresh_access_token
8+
from ....core import keyring
9+
from ..models import CredentialContext, CredentialResult
10+
from ..provider import CredentialProvider
811

912
logger = logging.getLogger(__name__)
1013

@@ -15,9 +18,6 @@ class KeyringProvider(CredentialProvider):
1518
name = "keyring"
1619

1720
def resolve(self, context: CredentialContext) -> CredentialResult | None:
18-
from ....cli.saml import refresh_access_token
19-
from ....core import keyring
20-
2121
if not keyring.should_use_keyring():
2222
return None
2323

‎cloudsmith_cli/core/credentials/session.py‎

Lines changed: 0 additions & 63 deletions
This file was deleted.

0 commit comments

Comments
 (0)