Skip to content
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

Add migration for new cyberark plugin names #13692

Merged
merged 2 commits into from
Mar 20, 2023

Conversation

obaranov
Copy link
Contributor

@obaranov obaranov commented Mar 15, 2023

SUMMARY

Fixed issue with the migration of the new names: #13669

ISSUE TYPE
  • Bug, Docs Fix or other nominal change
COMPONENT NAME
  • API
AWX VERSION
awx: 21.13.1.dev64+g268f884e92
ADDITIONAL INFORMATION

@AlanCoding
Copy link
Member

I think you're close here, with the addition of an .order_by('created') maybe. Needs testing.

I like that a lot better than having to reference specific names. In fact, this might be something that can be written generally, and applied to all managed types (instead of 2 operations).

Should add logging, in case a change is made, it is important to alert the user to it for auditing.

@obaranov obaranov force-pushed the cyberark-migration branch 4 times, most recently from 391a733 to 9e9c982 Compare March 17, 2023 12:52
Copy link
Member

@AlanCoding AlanCoding left a comment

Choose a reason for hiding this comment

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

This is a good near-term fix, but longer-term I would like to followup and adjust the logic inside of CredentialType.setup_tower_managed_defaults so that we won't need to do this again. Ideally, we could do this the next time that a credential type name is changed.

@obaranov
Copy link
Contributor Author

I think you're close here, with the addition of an .order_by('created') maybe. Needs testing.

I like that a lot better than having to reference specific names. In fact, this might be something that can be written generally, and applied to all managed types (instead of 2 operations).

Should add logging, in case a change is made, it is important to alert the user to it for auditing.

I added logging and reduced everything to one operation. I also checked that existing credentials were properly migrated after upgrade:
image

Also added upgrade API tests here: https://github.com/ansible/tower-qa/pull/8021

@obaranov obaranov merged commit 46227f1 into ansible:devel Mar 20, 2023
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.

None yet

2 participants