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

tower: modify tower_credential module to support custom cred types (ansible#54493) #54503

Open
wants to merge 2 commits into
base: devel
from

Conversation

Projects
None yet
3 participants
@stana
Copy link

stana commented Mar 28, 2019

SUMMARY

In addition to built-in credential types, Tower also supports creation of custom credential types.

Existing tower_credential module does not allow us to reference and create credential of custom type.

This change will allow for management of credentials of custom credential type.

ISSUE TYPE
  • Bugfix Pull Request
  • Docs Pull Request
  • Feature Pull Request
  • New Module Pull Request
COMPONENT NAME

tower_credential module

ADDITIONAL INFORMATION

Existing tower_credential module only supports built-in credential types specified using the 'kind' module param. These are built-in cred types (valid choices) supported -

["ssh", "vault", "net", "scm", "aws", "vmware", "satellite6", "cloudforms", "gce", "azure_rm", "openstack", "rhv", "insights", "tower"]

This change will allow for an extra 'kind' param choice value - 'cloud' (as in Tower, custom credentials types can only be defined as 'cloud' kind). If credential kind param has value of 'cloud', the new param 'credential_type' will be required (used to specify custom credential type name). See below for example.

This change is back-compatible - i.e. when any other built-in 'kind' is specified, credential_type param is not needed and logic is unchanged.

    - name: Credential
      tower_credential:
        name: "ACME Cred"
        organization: Foo
        kind: cloud
        credential_type: ACME
        ...

@ansibot

This comment has been minimized.

@ansibot

This comment has been minimized.

Copy link
Contributor

ansibot commented Mar 28, 2019

The test ansible-test sanity --test validate-modules [explain] failed with 1 error:

lib/ansible/modules/web_infrastructure/ansible_tower/tower_credential.py:0:0: E309 version_added for new option (credential_type) should be '2.8'. Currently StrictVersion ('0.0')

click here for bot help

@ansibot

This comment has been minimized.

Copy link
Contributor

ansibot commented Mar 28, 2019

@stana this PR contains the following merge commits:

Please rebase your branch to remove these commits.

click here for bot help

stana added some commits Mar 28, 2019

@stana stana force-pushed the stana:tower-custom-cred-module branch to 29dcdea Mar 28, 2019

choices: ["ssh", "vault", "net", "scm", "aws", "vmware", "satellite6", "cloudforms", "gce", "azure_rm", "openstack", "rhv", "insights", "tower", "cloud"]
credential_type:
description:
- Non built-in custom credential type name.

This comment has been minimized.

Copy link
@AlanCoding

AlanCoding Mar 28, 2019

Member

built-in types also have a credential type name. Treating credential_type as a special case for custom credentials doesn't jive great with the design goals.

I wonder if we could make either "kind" or "credential_type" required (irrelevant of custom vs. built-in types), and manage to get by that way. @ryanpetrello do you have any thoughts?

This comment has been minimized.

Copy link
@stana

stana Mar 29, 2019

Author

Thanks for reviewing the request. Seems like support for legacy v1 Credential model, when only the "kind" field was used to identify credential type (from a static list of types) complicates the design. The latest Credential implementation references credential type record (uniquely identified by credential type name, kind and organization). Seems like kind is required across both versions.

Depending on tower-cli credential type fetching logic, maybe an option to let tower-cli handle this complexity and just pass it optional credential type name, if provided. This way module would not need to handle differences between versions and built-in vs custom types.

This comment has been minimized.

Copy link
@AlanCoding

AlanCoding Mar 29, 2019

Member

The latest Credential implementation references credential type record (uniquely identified by credential type name, kind and organization). Seems like kind is required across both versions.

The v1 "kind" field is not the same as the v2 "kind" field. In v1, "kind" is a field on credentials and indicates something that corresponds to the v2 credential type (like aws, vmware, etc.). In v2, "kind" is a field on credential types, and only has a small handful of choices (like ssh, cloud, vault).

Seems like support for legacy v1 Credential model, when only the "kind" field was used to identify credential type (from a static list of types) complicates the design.

Certainly agree that the issue complicates the design. Removal of v1 is near-term in AWX:

ansible/awx#3413

Depending on tower-cli credential type fetching logic, maybe an option to let tower-cli handle this complexity and just pass it optional credential type name, if provided.

Based on the prior points, this looks like it could involve abusing the "kind" field, and doing this would be very confusing because of those two different functions. I'm sorry I don't have answers here.

@ansibot ansibot removed the needs_triage label Mar 28, 2019

@ansibot ansibot added the stale_ci label Apr 6, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.