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

For all tasks add in the 'id' field in addition to display name #601

Open
mbwhite opened this issue May 26, 2022 · 1 comment
Open

For all tasks add in the 'id' field in addition to display name #601

mbwhite opened this issue May 26, 2022 · 1 comment

Comments

@mbwhite
Copy link
Collaborator

mbwhite commented May 26, 2022

Today, the 'name' of components, such CAs, is really the 'display_name'. there is an assumption that this is unique and is suitable for lookup.

However there are cases where this is problematic, if a CA already has the name 'MyCA' requesting a new one, will create a CA with display name 'MyCA', but with a new id 'MyCA1' . When this is then referred to later, it's not possible to accurately locate the correct CA.

Plan:

  • Add ID into all the modules that support creation. Use that if it is supplied in the ID field (not mandatory in the current implementation)

  • Allow ID to be used in modules, when looking up components - prefer this to using display_name

  • For those that do fall back to display_name, then if multiple entries are returned, this will be considered an error. This could be considered a breaking change, as something that notionally worked before would fail. However as soon as the wrong component is chosen, the code has started down a (very likely) error path. Therefore best to fail fast.

  • Ensure that if created via just display_name, the returned ID is available for use later. It should be returned today but is not clearly documented as such.

@mbwhite
Copy link
Collaborator Author

mbwhite commented Jun 9, 2022

Documentation

From the Ansible module

id_name_mapping:
    description:
        - C(as_set) the values of name and id are used exactly as they have been passed in (id of "" is removed and not used)
        - C(id_is_name) the id value is taken directly from the name as is. This may limit the format of the name as there are restrictions on the id length
        - C(id_from_name) this id is generated from the name.
    type: str
    choices:
        - as_set
        - id_is_name
        - id_from_name

id:
    description:
        - The unique identifier of the certifcate authority. Will be required in a future release.
    type: str
    required: false

Notes

The name is really the 'Display Name' of the created node; it does not have to be unique; this can cause issues if the same script is rapidly re-run, so the whilst the first script is executing, a second is run. This will end up with 2 CAs being created - with the same display name but different ids. This causes confusion later - as the usual way of referring to any node is via DisplayName.

There is now an 'id' parameter that can be set this will be mapped directly to the 'id' field. This is optional and not specifying means everything will work as before. To assist in migration to using the 'id' field more, there is a second parameter id_name_mapping.

The default of this is as-set so the name, and if set the id, are used exactly as is.
id_is_name will use the Display Name as the id without modifications. This does mean that the display name will be limited to the same rules as the id. (for IBP this is 3-8 characters, lowercase & numbers. first character to be a letter)
id_from_name will generate the ID from the DisplayName (truncating and lowercasing as needed).

UPDATE: Since writing confirmed that the limit on IDs is up to 64 characters. therefore propose to change the id_from_name to take the first (32?) display name characters, and append a random suffix,.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: No status
Development

No branches or pull requests

1 participant