Skip to content

Migrate AAA table per-command authorization in db_migrator #3296

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

liuh-80
Copy link
Contributor

@liuh-80 liuh-80 commented Apr 29, 2024

Migrate AAA table per-command authorization in db_migrator

Why I did it

per-command AAA need enable in warm-upgrade case

How I did it

Add code to migrate per-command aunthorization

How to verify it

Pass all test case.
Add new test case.

Which release branch to backport (provide reason below if selected)

N/A

Description for the changelog

Migrate AAA table per-command authorization in db_migrator

A picture of a cute animal (not mandatory but encouraged)

@liuh-80 liuh-80 marked this pull request as ready for review April 29, 2024 08:00
@liuh-80 liuh-80 requested review from qiluo-msft and ganglyu April 29, 2024 08:00
@liuh-80
Copy link
Contributor Author

liuh-80 commented Apr 29, 2024

This PR is based on per-command accounting migrate PR: #3284

@qiluo-msft qiluo-msft requested a review from vaibhavhd May 8, 2024 06:50
tacplus_config = self.configDB.get_entry('TACPLUS', 'global')
if 'passkey' in tacplus_config and '' != tacplus_config.get('passkey'):
authorization = self.configDB.get_entry('AAA', 'authorization')
if not authorization:
Copy link
Contributor

@qiluo-msft qiluo-msft May 8, 2024

Choose a reason for hiding this comment

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

authorization

Could you provide a table to analyze the old_config, new_config, actions for all the situations?
#Closed

Copy link
Contributor Author

@liuh-80 liuh-80 May 11, 2024

Choose a reason for hiding this comment

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

According to your comments in #3284:

qiluo-msft 2 weeks ago
The old image's enablement does not matter. As long as the required passkey is there, and intention is to enable, then we should enable.

So, we only check the new config:

New_config Action
Passkey does not exist Not enable
Passkey exist, Authorization enable Enable
Passkey exist, Authorization not enable Not enable

self.configDB.set_entry("AAA", "authorization", authorization_new)
log.log_info('Migrate AAA authorization: {}'.format(authorization_new))
else:
# If no passkey, setup per-command authorization will block remote user command
Copy link
Contributor

@qiluo-msft qiluo-msft May 12, 2024

Choose a reason for hiding this comment

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

If no passkey

As the designed in your table, you should disable Authorization. However, the current implement is just log and skip. #Closed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed, disable authorization by delete entry.

@liuh-80 liuh-80 merged commit 61d0ec9 into sonic-net:master May 15, 2024
mssonicbld pushed a commit to mssonicbld/sonic-utilities that referenced this pull request May 21, 2024
…#3296)

Migrate AAA table per-command authorization in db_migrator

#### Why I did it
    per-command AAA need enable in warm-upgrade case

#### How I did it
    Add code to migrate per-command aunthorization

#### How to verify it
    Pass all test case.
    Add new test case.

#### Which release branch to backport (provide reason below if selected)
    N/A

#### Description for the changelog
    Migrate AAA table per-command authorization in db_migrator

#### A picture of a cute animal (not mandatory but encouraged)
@mssonicbld
Copy link
Collaborator

Cherry-pick PR to 202311: #3330

mssonicbld pushed a commit that referenced this pull request May 21, 2024
Migrate AAA table per-command authorization in db_migrator

#### Why I did it
    per-command AAA need enable in warm-upgrade case

#### How I did it
    Add code to migrate per-command aunthorization

#### How to verify it
    Pass all test case.
    Add new test case.

#### Which release branch to backport (provide reason below if selected)
    N/A

#### Description for the changelog
    Migrate AAA table per-command authorization in db_migrator

#### A picture of a cute animal (not mandatory but encouraged)
arfeigin pushed a commit to arfeigin/sonic-utilities that referenced this pull request Jun 16, 2024
…#3296)

Migrate AAA table per-command authorization in db_migrator

#### Why I did it
    per-command AAA need enable in warm-upgrade case

#### How I did it
    Add code to migrate per-command aunthorization

#### How to verify it
    Pass all test case.
    Add new test case.

#### Which release branch to backport (provide reason below if selected)
    N/A

#### Description for the changelog
    Migrate AAA table per-command authorization in db_migrator

#### A picture of a cute animal (not mandatory but encouraged)
nmoray pushed a commit to nmoray/sonic-utilities that referenced this pull request Jun 25, 2025
…#3296)

Migrate AAA table per-command authorization in db_migrator

#### Why I did it
    per-command AAA need enable in warm-upgrade case

#### How I did it
    Add code to migrate per-command aunthorization

#### How to verify it
    Pass all test case.
    Add new test case.

#### Which release branch to backport (provide reason below if selected)
    N/A

#### Description for the changelog
    Migrate AAA table per-command authorization in db_migrator

#### A picture of a cute animal (not mandatory but encouraged)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants