Skip to content

fix(cssc): 31670352 add check for empty token for authentication (1/n)#39

Merged
cegraybl merged 1 commit into
feature/cssc_extfrom
cegaybl/cssc_31670352_empty_token_1
Mar 19, 2025
Merged

fix(cssc): 31670352 add check for empty token for authentication (1/n)#39
cegraybl merged 1 commit into
feature/cssc_extfrom
cegaybl/cssc_31670352_empty_token_1

Conversation

@cegraybl
Copy link
Copy Markdown

This change is only to add check for empty token.
A following PR will be made to update the oras package. Since there are substantial changes to the client usage.

Adding a centralized check for every token we retrieve from the registry and improve on error logging

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds an empty token check during authentication and refines error handling in the ACR token retrieval. Key changes include:

  • Switching from subprocess.Popen to subprocess.run with improved output handling.
  • Inserting a check to detect an empty token and raising a specific error.
  • Adjusting the exception handling logic for clearer error messaging.
Comments suppressed due to low confidence (2)

src/acrcssc/azext_acrcssc/helper/_ociartifactoperations.py:157

  • [nitpick] Consider using a higher log level (e.g., logger.error) instead of logger.debug when an empty token is detected, as this condition indicates a critical authentication failure.
if not token or token == "":

src/acrcssc/azext_acrcssc/helper/_ociartifactoperations.py:164

  • [nitpick] The exception block computes an 'unauthorized' condition but then always raises a generic AzCLIError; consider consolidating the error handling logic to provide a more consistent and clear error message based on the unauthorized case.
unauthorized = (error.stderr

Copy link
Copy Markdown

@Ruchii-27 Ruchii-27 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@cegraybl cegraybl merged commit 4173416 into feature/cssc_ext Mar 19, 2025
cegraybl added a commit that referenced this pull request Apr 8, 2025
…41)

This change is a continuation of the changes in [PR
39](#39)
Updates the `oras` packages to 0.2.25 to use a more recent version of
the package to avoid the "empty token" error when authenticating to the
registry.

---------

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@cegraybl cegraybl deleted the cegaybl/cssc_31670352_empty_token_1 branch April 23, 2025 17:10
cegraybl added a commit that referenced this pull request Apr 23, 2025
#39)

This change is only to add check for empty token.
A following PR will be made to update the oras package. Since there are
substantial changes to the client usage.

Adding a centralized check for every token we retrieve from the registry
and improve on error logging
cegraybl added a commit that referenced this pull request Apr 23, 2025
…41)

This change is a continuation of the changes in [PR
39](#39)
Updates the `oras` packages to 0.2.25 to use a more recent version of
the package to avoid the "empty token" error when authenticating to the
registry.

---------

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
cegraybl added a commit that referenced this pull request Apr 24, 2025
#39)

This change is only to add check for empty token.
A following PR will be made to update the oras package. Since there are
substantial changes to the client usage.

Adding a centralized check for every token we retrieve from the registry
and improve on error logging
cegraybl added a commit that referenced this pull request Apr 24, 2025
…41)

This change is a continuation of the changes in [PR
39](#39)
Updates the `oras` packages to 0.2.25 to use a more recent version of
the package to avoid the "empty token" error when authenticating to the
registry.

---------

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
mabelegba pushed a commit that referenced this pull request Nov 25, 2025
* Upstream Merge

* Merge from workload-orchestration : Added Example in Description (#34)

* Merge from workload-orchestration : Fixed Example

* Fixed Example for SolutionTemplateVersion

* Made Changes for command-change CI fix'

* Reset Version 1.0.0b1

* Added Bulk and Diagnostics Back (#35)

Co-authored-by: Atharva Udapure <audapure@microsoft.com>

* Added Bulk Solution Example (#36)

* Added Bulk and Diagnostics Back

* Changes

---------

Co-authored-by: Atharva Udapure <audapure@microsoft.com>

* Fixed Solution Template Linter Issue

* Added Integration Tests Framework for WorkloadOrchestration #37

Co-authored-by: Atharva Udapure <audapure@microsoft.com>

* Added ServiceName  (#38)

* Added Tests

* Made Changes on ServiceName

---------

Co-authored-by: Atharva Udapure <audapure@microsoft.com>

* Added Full Target Solution Tests (#39)

* Added Tests

* Made Changes on ServiceName

* Nit Changes

* Made Changes for ContextLookup

---------

Co-authored-by: Atharva Udapure <audapure@microsoft.com>

* Added E2E workflow Tests (#40)

* Added Tests

* Made Changes on ServiceName

* Nit Changes

* Made Changes for ContextLookup

* Added complete workflow

* Add Licence

* Added License

---------

Co-authored-by: Atharva Udapure <audapure@microsoft.com>

* Added Readme (#41)

Co-authored-by: Atharva Udapure <audapure@microsoft.com>

* Bulk Deployemnent LRO changes (#42)

* made changes

* Added change

* Added Changes in Test

* Added CLi Changes

* Added Setup

---------

Co-authored-by: Atharva Udapure <audapure@microsoft.com>

* Added Changes For Backward Compitablity (#43)

* Added Changes For Backward Compitablity

* Made changes

---------

Co-authored-by: Atharva Udapure <audapure@microsoft.com>

* Made changes (#44)

Co-authored-by: Atharva Udapure <audapure@microsoft.com>

* Target Operations API refractoring  (#47)

* Changes I made

* Make changes

* Made chnages

* Make changes

---------

Co-authored-by: Atharva Udapure <audapure@microsoft.com>

* Stable API 2025-06-01 (#48)

* Made changes

* Made Changes

---------

Co-authored-by: Atharva Udapure <audapure@microsoft.com>

* Change in ID in Targets (#50)

* Made changes

* changes

---------

Co-authored-by: Atharva Udapure <audapure@microsoft.com>

* Remove Id and Name from Target Review  (#51)

Co-authored-by: Atharva Udapure <audapure@microsoft.com>

* final bulk (#52)

Co-authored-by: Atharva Udapure <audapure@microsoft.com>

* Changes in ARG

* Added Linter Exception

* Added ITTests (#53)

Co-authored-by: Atharva Udapure <audapure@microsoft.com>

* Standardize CLI (#54)

* Made Changes

* Made Changes

* Made changes

* Added Change

* Made changes

* Made changes

* Made changes

* Made Changes

---------

Co-authored-by: Atharva Udapure <audapure@microsoft.com>

* Linter FIxes and Remove Preview

* Removed Resolved from CLI

* change in version

* made changes (#55)

Co-authored-by: Atharva Udapure <audapure@microsoft.com>

* Fixed history

---------

Co-authored-by: Atharva Udapure <audapure@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants