Add import support to consul_config_entry #319
Conversation
There was a problem hiding this comment.
👋 @remilapeyre
I had few questions about that one, specifically around partition and namespace handling.
| return nil, fmt.Errorf(`expected path of the form "<kind>/<name>" or "<kind>/<name>/<partition>/<namespace>"`) | ||
| } | ||
|
|
||
| d.SetId(fmt.Sprintf("%s-%s", kind, name)) |
There was a problem hiding this comment.
Shouldn't partition and namespace be added to the ID to ensure uniqueness?
There was a problem hiding this comment.
The ID does not need to be unique. For config entries there is no ID that exist so we can use any placeholder there. We have other resources that not have a unique ID either, there is some examples at: #318 (comment)
templates/resources/config_entry.md
Outdated
| `consul_config_entry` can be imported using the syntax `<kind>/<name>` if the | ||
| config entry is in the default partition and default namespace, or | ||
| `<kind>/<name>/<partition>/<namespace>` for config entries in a non-default | ||
| partition or namespace: |
There was a problem hiding this comment.
What about providing foo/bar/default/default is it equivalent to foo/bar?
There was a problem hiding this comment.
Yes it Is equivalent. Looking at it now it seems to me like <partition>/<namespace>/<kind>/<name> might make more sense here.
There was a problem hiding this comment.
I agree, that would be more inline with consul annotations around namespace and partition. Can we change it to reflect this?
Related to #316