-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Fix identity assignment when labels are changed #3104
Conversation
test-me-please |
1f4a11e
to
7a98127
Compare
test-me-please |
7a98127
to
6a5ed0d
Compare
test-me-please |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
@@ -1768,7 +1768,9 @@ func (e *Endpoint) identityLabelsChanged(owner Owner, myChangeRev int) error { | |||
return nil | |||
} | |||
|
|||
if e.SecurityIdentity != nil && string(e.SecurityIdentity.Labels.SortedList()) != string(newLabels.SortedList()) { | |||
if e.SecurityIdentity != nil && |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Am I missing the change here or is this just a cosmetic change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
!=
-> ==
daemon/state.go
Outdated
ep.SecurityIdentity.Labels = l | ||
|
||
identity, _, err := identityPkg.AllocateIdentity(ep.SecurityIdentity.Labels) | ||
l, _ := labels.FilterLabels(ep.OpLabels.AllLabels()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This also includes disabled labels. That doesn't seem right. I think you want to use e.OpLabels.IdentityLabels
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh, awesome, that's exactly what I want.
6a5ed0d
to
6888ffe
Compare
6888ffe
to
1ce2df6
Compare
test-me-please |
Summary of changes:
Reported by @mrdima