Skip to content

Warn instead of fail on failure to validate against DB #2668

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.

Sign up for GitHub

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

Closed
mheon wants to merge 2 commits into from
Closed

Warn instead of fail on failure to validate against DB #2668

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
de8a6b8
Warn instead of fail on failure to validate against DB
mheon Mar 15, 2019
31b12dc
Don't overwrite storage defaults when running as root
mheon Mar 15, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions libpod/boltdb_state_internal.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,10 +146,11 @@ func validateDBAgainstConfig(bucket *bolt.Bucket, fieldName, runtimeValue string
return nil
}

// If DB value is the empty string, check that the
// runtime value is the default
if string(keyBytes) == "" && defaultValue != "" &&
runtimeValue == defaultValue {
// If DB value is the empty string, we ought to be fine.
// (Though I'm honestly not sure how we got this far
// with an empty-string DB config)
if string(keyBytes) == "" {
Copy link
Member

@rhatdan rhatdan Mar 15, 2019

Choose a reason for hiding this comment

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

Should we update the data with the correct value?

Copy link
Member Author

@mheon mheon Mar 15, 2019

Choose a reason for hiding this comment

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

The problem is that we don't really know the correct value here - the DB is supposed to know, and it doesn't, so we can only hope that we're using the right storage driver.

Copy link
Member Author

@mheon mheon Mar 15, 2019

Choose a reason for hiding this comment

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

I think I've figured out why we're entering "" in the first place though. Going to fix that next.

logrus.Warnf("Database stored empty configuration for %s - unable to validate", fieldName)
return nil
}

Expand Down
3 changes: 3 additions & 0 deletions pkg/util/utils.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -297,6 +297,9 @@ func GetDefaultStoreOptions() (storage.StoreOptions, error) {
err error
)
storageOpts := storage.DefaultStoreOptions
if !rootless.IsRootless() {
Copy link
Member

@rhatdan rhatdan Mar 16, 2019

Choose a reason for hiding this comment

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

This code makes little sense. Above you set storageOpts to DefaultStoreOptions, then you overwrite it in both root and rootless mode.

storageOpts = storage.StoreOptions{}
}
if rootless.IsRootless() {
storageOpts, err = GetRootlessStorageOpts()
if err != nil {
Expand Down