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

Adding Credential Input Source Export for Exporting Credentials using awxkit #14798

Open
wants to merge 19 commits into
base: devel
Choose a base branch
from

Conversation

neevnuv
Copy link

@neevnuv neevnuv commented Jan 24, 2024

SUMMARY

When exporting credentials using awxkit, if the credential had input sources, they would not be exported.
This PR will add the ability to export those values and import them afterwards. #14738

ISSUE TYPE
  • New or Enhanced Feature
COMPONENT NAME
  • Collection
  • CLI
AWX VERSION
23.6.0
ADDITIONAL INFORMATION
Relates to the issue #14738 

@neevnuv neevnuv requested a review from jbradberry February 8, 2024 21:49
@dmzoneill dmzoneill marked this pull request as draft February 20, 2024 11:54
@neevnuv neevnuv marked this pull request as ready for review February 27, 2024 16:28
@dmzoneill dmzoneill marked this pull request as draft February 28, 2024 07:15
@neevnuv
Copy link
Author

neevnuv commented Feb 28, 2024

Hello, I finished working on this PR.
Would love for a review, I am not sure if to keep as a draft or not.

@jbradberry
Copy link
Contributor

@neevnuv Ok, here's what I think. After sitting down and experimenting with an input source for a bit, I see that the related endpoint off of api/v2/credential/<id>/ is a bit odd and won't fit into the framework of DEPENDENT_EXPORT without refactoring how that all works or introducing another edge case like we did with workflow approvals.

Instead, I think you should just add CredentalInputSource to the EXPORTABLE_RESOURCES list, and give the page type a proper NATURAL_KEY. This information ought to tell you what the natural key should be: https://github.com/ansible/awx/blob/devel/awx/main/models/credential/__init__.py#L1243

Yes, this has the downside that users will need to remember to explicitly export their credential input sources (unless they are exporting everything), but I think it will be simpler all around.

@neevnuv
Copy link
Author

neevnuv commented Apr 14, 2024

As of now, I made it possible to access the credential_input_sources using:

awx export --credential_input_sources

and allow the credentials that use input sources to export them when using:

awx export --credential

Now I am researching on payload functions and how they are used.

@@ -48,6 +49,7 @@
('Inventory', 'Host'),
('Inventory', 'Label'),
('WorkflowJobTemplateNode', 'WorkflowApprovalTemplate'),
('Credential', 'CredentialInputSource'),
Copy link
Contributor

Choose a reason for hiding this comment

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

You probably don't want this here.

Copy link
Author

Choose a reason for hiding this comment

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

won't this allow to export the credential input sources when imoprting a credential with an input source?

Copy link
Contributor

Choose a reason for hiding this comment

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

No. As I explained before, DEPENDENT_EXPORT doesn't quite work with how this relationship is being exposed in the API. You'll have to leave this out, and in order to correctly export a user will need to do both --credentials and --credential_input_sources.

@jbradberry jbradberry marked this pull request as ready for review April 19, 2024 15:53
@neevnuv neevnuv force-pushed the devel branch 2 times, most recently from 6908d92 to 888e3b2 Compare June 7, 2024 12:55
@fosterseth
Copy link
Member

fosterseth commented Jun 10, 2024

I keep getting this when importing

Traceback (most recent call last):
  File "/home/sbf/awx/awxkit/awxkit/cli/__init__.py", line 25, in run
    cli.parse_resource()
  File "/home/sbf/awx/awxkit/awxkit/cli/client.py", line 152, in parse_resource
    self.resource = parse_resource(self, skip_deprecated=skip_deprecated)
                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/sbf/awx/awxkit/awxkit/cli/resource.py", line 222, in parse_resource
    response = command.handle(client, parser)
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/sbf/awx/awxkit/awxkit/cli/resource.py", line 145, in handle
    client.v2.import_assets(data)
  File "/home/sbf/awx/awxkit/awxkit/api/pages/api.py", line 422, in import_assets
    for resource in self._dependent_resources():
  File "/home/sbf/awx/awxkit/awxkit/api/pages/api.py", line 267, in _dependent_resources
    for page_cls in itertools.chain(*has_create.page_creation_order(*data_pages)):
                                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/sbf/awx/awxkit/awxkit/api/mixins/has_create.py", line 96, in page_creation_order
    actual_order = separate_async_optionals(order)
                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/sbf/awx/awxkit/awxkit/api/mixins/has_create.py", line 68, in separate_async_optionals
    if dependency in compared.optional_dependencies:
                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AttributeError: type object 'CredentialInputSource' has no attribute 'optional_dependencies'

any idea what is going on?

@jbradberry
Copy link
Contributor

@neevnuv The above traceback is because you don't have the HasCreate mixin class as one of the bases for your new page type. Please include everything in api/mixins/ that seems relevant for this type of API endpoint.

@jbradberry jbradberry requested review from fosterseth and jbradberry and removed request for jbradberry June 10, 2024 20:46
@neevnuv
Copy link
Author

neevnuv commented Jun 13, 2024

Thanks for the feedback, I added only HasCreate.

Reasons:
HasInstanceGroups - credential_input_sources don't have instanceGroups.
HasNotifications - credential_input_sources don't have notifications.
HasStatus - credential_input_sources don't have status.
HasSurvey - credential_input_sources don't have survies.
HasVariables - credential_input_sources don't have inventories variables.
HasCopy - doesn't have an option to copy.

@jbradberry
Copy link
Contributor

cc @fosterseth

Copy link

@lukewestervelt
Copy link

@fosterseth @jbradberry @djyasin

I'm currently encountering the same issue (#14738) with TSS SecureVault secrets. AWX instance credentials are setup to pull username/passwords from TSS SecureVault instead of storing in plaintext/encrypted in the username/password fields.

Was going through export/import AWX process, and attempting to inject secrets back into credentials during the import process. Attempted to add the TSS JSON inside of the inputs block like so:

      "inputs": {
        "password": {
          "secret_field": "password",
          "secret_id": "#####"
        },
        "username": {
          "secret_field": "username",
          "secret_id": "#####"
        }
      },

But met with this error message:
ERROR:awxkit.api.pages.api:/api/v2/credentials/ "GitHub PAT": Bad Request (400) received

After diving into the AWX instance with credentials objects that are setup correctly, inputs and input_sources block are missing even before export/import is attempted:

// using awx-manage shell_plus for below- https://github.com/ansible/awx/blob/989a4387df6cf802a0f00c0251669ff6116889c0/docs/credentials/extract_credentials.md?plain=1#L23C1-L23C105

print(Credential.objects.get(name="GitHub PAT").inputs)
{}
print(Credential.objects.get(name="GitHub PAT").input_sources)
main.CredentialInputSource.None

Seems to me that this PR would possibly address this issue. Are there plans to merge this? Any other guidence would be much appreciated, thanks!

@jbradberry
Copy link
Contributor

@lukewestervelt I'm afraid I'm no longer on this team and have no further say in what happens on this project. Hopefully @fosterseth will be able to make that call. My recollection was that this PR is ready or close to it.

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.

4 participants