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

SecretsGroup feature #1042

Merged
merged 15 commits into from
Nov 5, 2021
Merged

Conversation

glennmatthews
Copy link
Contributor

Incremental work towards #541, building atop #868.

This adds a SecretsGroup model that is used to aggregate one or more Secrets and assign an access type (SSH, HTTP, gNMI, etc.) and a secret type (username, password, token, etc.) to each Secret within the context of the group.

The Device and GitRepository models now have the option to associate to a defined SecretsGroup. A GitRepository can reference a username/token within its assigned group to access a private Git repository. The built-in NAPALM integration will use a username/password/secret within a device's assigned group in preference to the (still supported!) NAPALM_USERNAME and NAPALM_PASSWORD global configs.

Includes documentation updates, UI views, REST API, and test coverage. Sorry for the size of this PR as a result!

@glennmatthews
Copy link
Contributor Author

Some bugs to fix:

  • Leftover "Type" column in SecretsTable
  • Secret used multiple times in one group (password/secret for NAPALM) causes group to appear multiple times in the Secret detail view.

Copy link
Contributor

@jathanism jathanism left a comment

Choose a reason for hiding this comment

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

Everything is working as intended. I do think it would be good to see some integration tests for this after you adjust the CSS!

Comment on lines +801 to +806
# TODO: it would be **awesome** if we could create/update SecretsGroupAssociations
# alongside creating/updating the base SecretsGroup, but since this is a ManyToManyField with
# a `through` table, that appears very non-trivial to implement. For now we have this as a
# read-only field; to create/update SecretsGroupAssociations you must make separate calls to the
# api/extras/secrets-group-associations/ REST endpoint as appropriate.
secrets = NestedSecretsGroupAssociationSerializer(source="secretsgroupassociation_set", many=True, read_only=True)
Copy link
Contributor

Choose a reason for hiding this comment

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

It IS possible by using nested serializers, which would allow for this to happen in a single call. Was that an approach you tried? (It might also involve a combination using the writable nested serializers pattern by overloading .create() and .update()?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I spent a good chunk of time on it and wasn't able to get it to work to my satisfaction. Partly I just don't have a good example to work from (we have plenty that are using ManyToMany fields, but not with a through table with its own additional required fields, where we need to represent the through table, not the target table) and the DRF docs don't provide any details on how to implement this case other than just saying it's possible. I think you're right that we'd need to write custom create/update functions.

nautobot/docs/user-guides/git-data-source.md Outdated Show resolved Hide resolved
glennmatthews and others added 2 commits November 1, 2021 14:42
@glennmatthews glennmatthews merged commit 13351f4 into gfm-secrets Nov 5, 2021
@glennmatthews glennmatthews deleted the gfm-secrets-additional-models branch November 5, 2021 17:58
glennmatthews added a commit that referenced this pull request Nov 15, 2021
… Groups (#868)

* Initial model, UI, and REST API for Secrets

* Secrets providers API, initial TextFile and EnvironmentVariable provider implementations (#887)

* Add Secret.value property, add EnvironmentVariable provider, add dummy-plugin Constant provider, add tests

* Add TextFileSecretProvider

* Add docs

* Improve display of secret providers in the UI

* Refactor SecretsProvider registration to use the Nautobot registry instead of python entry_points

* Refactor slightly

* Add ability for secrets providers to define an HTML form for parameter inputs

* Fix default value for JSONField and add error handling in JS

* Add username_secret and token_secret support to GitRepository

* Docs updates

* Review feedback - add description field, etc.

* Revise secrets docs; add SecretError exceptions instead of returning None on various failures

* One of these days I'll remember to run flake8 before pushing

* Review comments

* SecretsGroup feature (#1042)

* WIP

* More WIP

* WIP remove SecretType model

* Such WIP. Wow

* WIP: working secretsgroup-edit UI

* More WIP

* Change Category/Meaning to Access Type/Secret Type

* Add SecretsGroup key to Device model; get tests passing

* Add test coverage for REST API and filters

* Add SecretsGroup view tests

* Linting fixes

* Docs updates

* Cleanup leftover SecretType cruft

* Update nautobot/docs/user-guides/git-data-source.md

Co-authored-by: Jathan McCollum <jathan@gmail.com>

* Fix egregious issues

Co-authored-by: Jathan McCollum <jathan@gmail.com>

* Support Jinja2 templating of secret parameters (#1058)

* Support Jinja2 templating of secret parameters

* Add secrets providers to plugin detail view

* Doc updates

* Include SecretsGroupAssociation in GraphQL

* Move 'Secrets' to a top-level menu

* Don't try to sort `SecretsProvider` class objects in plugin config features registry (#1065)

* Fix TypeError when trying to sort `SecretsProvider` class objects
* Don't sort `secrets_providers` when added to features.

* Add release-note content for Secrets

* Update nautobot/extras/views.py

Co-authored-by: John Anderson <lampwins@gmail.com>

* Change FK to SecretsGroup behavior to SET_NULL

* Use render_jinja2() in rendered_parameters()

Co-authored-by: Jathan McCollum <jathan@gmail.com>
Co-authored-by: John Anderson <lampwins@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants