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

Case insensitive identity names #5404

Merged
merged 8 commits into from
Oct 19, 2018
Prev Previous commit
Next Next commit
address review feedback
  • Loading branch information
vishalnayak committed Oct 11, 2018
commit 328119fc1a2c568d963825ce2d8c2f67d9754ec0
8 changes: 4 additions & 4 deletions vault/identity_store_util.go
Original file line number Diff line number Diff line change
@@ -45,10 +45,11 @@ func (c *Core) loadIdentityStoreArtifacts(ctx context.Context) error {
// If it succeeds, all is well
return nil
case err != nil && !errwrap.ContainsType(err, errDuplicateIdentityName):
Copy link
Member

Choose a reason for hiding this comment

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

I don't think ContainsType does what you think it does here, which makes me wonder if this works properly. ContainsType is supposed to be used to validate a typed error, which means this will likely match against any error generated via errors.New. You probably want .Contains()

Copy link
Member Author

Choose a reason for hiding this comment

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

Wow. This is a nice observation. Fixed. But the code was working properly because the code expecting a true and ContainsType (as you said) was returning true. There wasn't a different error there to catch. I tested it by overwriting the err returned from loadFunc and verified that its getting caught properly now.

// Return all errors except one
return err
}

c.identityStore.logger.Warn("enabling case sensitive identity names")

// Set identity store to operate on case sensitive identity names
c.identityStore.disableLowerCasedNames = true

@@ -60,9 +61,8 @@ func (c *Core) loadIdentityStoreArtifacts(ctx context.Context) error {
return err
}

c.identityStore.logger.Warn("enabling case sensitive identity names")

// Load everything from the beginning.
// Attempt to load identity artifacts once more after memdb is reset to
// accept case sensitive names
return loadFunc(ctx)
}