Skip to content

Conversation

@HorjuRares
Copy link
Contributor

@HorjuRares HorjuRares commented Jun 3, 2025

Issues: #69

Definition of Done

The PR shall be merged only if all items mentioned in CONTRIBUTING.md have been followed. In case an item is not applicable as described, please provide a short explanation in the description.

@HorjuRares HorjuRares self-assigned this Jun 3, 2025
@HorjuRares HorjuRares requested a review from GabyUnalaq June 3, 2025 10:18
@HorjuRares HorjuRares added the ready for review Waiting for a response from contributor label Jun 3, 2025
@GabyUnalaq GabyUnalaq linked an issue Jun 4, 2025 that may be closed by this pull request
Copy link
Contributor

@GabyUnalaq GabyUnalaq left a comment

Choose a reason for hiding this comment

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

The changes do not fix the issue.

How I tested

I've run the hello_ankaios example with a changed manifest:
examples/manifest.yaml:

...
    controlInterfaceAccess:
      allowRules:
        - type: StateRule
          operation: ReadWrite
          filterMask:
            - "desiredState.workloads"
            - "workloadStates"
...

And run it in dev mode: ./run_example.sh hello_ankaios dev

Output before:

image

Output after

image

Suggestions

By implementing the suggestion from @krucod3 from the issue description, the second line is deleted, but the first one remains: Error while trying to get the state: Access denied.

Hint: This line comes from the Ankaios.get_state method, and this should be suppressed as well in this case.

@HorjuRares
Copy link
Contributor Author

image
Is this proper behaviour?

@GabyUnalaq
Copy link
Contributor

There should not be any message regarding the access denied.

@GabyUnalaq GabyUnalaq removed the ready for review Waiting for a response from contributor label Jun 5, 2025
@HorjuRares HorjuRares added the ready for review Waiting for a response from contributor label Jun 6, 2025
Copy link
Contributor

@GabyUnalaq GabyUnalaq left a comment

Choose a reason for hiding this comment

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

There are 2 cases for when the check if successful:

  • The CompleteState with only the apiVersion is returned, thus the connection is fine.
  • The request is blocked due to not enough privileges, thus the connection is fine.

In all other cases (ConnectionClosed or ProtocolException), the connection check fails. Because the flag never gets set to True if the apiVersion is successfully returned, the error line in get state for subsequent requests never get's logged in case of a privilege issue.

Copy link
Contributor

@GabyUnalaq GabyUnalaq left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@GabyUnalaq GabyUnalaq merged commit 8c75d17 into main Jun 10, 2025
21 checks passed
@GabyUnalaq GabyUnalaq deleted the 69_connection_check branch June 10, 2025 10:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready for review Waiting for a response from contributor

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Connection check fails on insufficient privileges

3 participants