Enable filtering entry id's by prefix #325
Conversation
This enables peaceful coexistence of multiple spire-controller-managers or other managers and manual entries in the same spire-server. Also provides a cleanup option for migration. Signed-off-by: Kevin Fox <[email protected]>
Signed-off-by: Kevin Fox <[email protected]>
cmd/main.go
Outdated
| if retval.ctrlConfig.EntryIDPrefix != "" && retval.ctrlConfig.EntryIDPrefix[len(retval.ctrlConfig.EntryIDPrefix)-1] != "."[0] { | ||
| retval.ctrlConfig.EntryIDPrefix += "." | ||
| } | ||
| if retval.ctrlConfig.EntryIDPrefixCleanup != nil && *retval.ctrlConfig.EntryIDPrefixCleanup != "" && (*retval.ctrlConfig.EntryIDPrefixCleanup)[len(*retval.ctrlConfig.EntryIDPrefixCleanup)-1] != "."[0] { |
There was a problem hiding this comment.
This is pretty unwieldy. I'd add a addDotSuffix function or something that took in a string and appended a . if-and-only-if the string is non-empty and doesn't already have the suffix. Then this could be simplified to:
| if retval.ctrlConfig.EntryIDPrefixCleanup != nil && *retval.ctrlConfig.EntryIDPrefixCleanup != "" && (*retval.ctrlConfig.EntryIDPrefixCleanup)[len(*retval.ctrlConfig.EntryIDPrefixCleanup)-1] != "."[0] { | |
| if retval.ctrlConfig.EntryIDPrefixCleanup != nil { | |
| *retval.ctrlConfig.EntryIDPrefixCleanup = addDotSuffix(*retval.ctrlConfig.EntryIDPrefixCleanup) | |
| } |
It could also be used unconditional above with EntryIDPrefix.
cmd/main.go
Outdated
| @@ -179,6 +179,21 @@ func parseConfig() (Config, error) { | |||
| retval.reconcile = *retval.ctrlConfig.Reconcile | |||
| } | |||
|
|
|||
| if retval.ctrlConfig.EntryIDPrefix != "" && retval.ctrlConfig.EntryIDPrefixCleanup != nil && retval.ctrlConfig.EntryIDPrefix == *retval.ctrlConfig.EntryIDPrefixCleanup { | |||
There was a problem hiding this comment.
I think you want to do this equality comparison after you've normalized these values with the dot suffix.
There was a problem hiding this comment.
Normalized and moved to reduce comparisons
| } | ||
|
|
||
| printCleanup := "<unset>" | ||
| if retval.ctrlConfig.EntryIDPrefixCleanup != nil { |
There was a problem hiding this comment.
I think this would all be a little easier to read if there was if block that handled all of the code related to EntryIDPrefixCleanup being non-nil. Right now it's spread across four blocks and it's a little hard to track.
pkg/spireentry/reconciler.go
Outdated
| if strings.Contains(entry.ID, "/") { | ||
| return false, false | ||
| } | ||
| return false, true |
There was a problem hiding this comment.
Dot is the separator between prefix, correct? Also this can be simplified.
| if strings.Contains(entry.ID, "/") { | |
| return false, false | |
| } | |
| return false, true | |
| return false, strings.Contains(entry.ID, ".") { |
There was a problem hiding this comment.
Close... it needs to be negated though... Will rework it that way though to be this pattern.
Co-authored-by: Andrew Harding <[email protected]> Signed-off-by: kfox1111 <[email protected]>
Signed-off-by: Kevin Fox <[email protected]>
Signed-off-by: Kevin Fox <[email protected]>
This enables peaceful coexistence of multiple spire-controller-managers or other managers and manual entries in the same spire-server. Also provides a cleanup option for migration.
fixes: spiffe/spire#4898