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

RFC: Simplify config handling #1819

Closed
dominikschulz opened this issue Feb 28, 2021 · 6 comments · Fixed by #2395
Closed

RFC: Simplify config handling #1819

dominikschulz opened this issue Feb 28, 2021 · 6 comments · Fixed by #2395
Milestone

Comments

@dominikschulz
Copy link
Member

The current config implementation is pretty strict so it can provide proper backwards compatibility.

However this makes it harder to edit the configuration manually since we don't allow extra fields and don't support comments, cf. #1654

Should we change the config format to a simple key-value format (ini-style, no YAML) that supports comments?

It's not trivial since then programmatic changes, e.g. through gopass config foo bar become more difficult and we'd need to figure out backwards compatibility. But I wonder if there is any need for that at all.

@AnomalRoil
Copy link
Member

We might also want to discuss #1567 since we're discussing config handling :)

In my opinion:

  • We should support a config file per store, that's stored at the root of the said store and fallback to the "main" config file if the store doesn't have a specific config. (This can be annoying in teams, since the config file per store might be shared? Maybe have a setting in the main one to disable secondary config files?) Ideally the logic to handle it should be the same, it would just be its location that would influence which file is used.

  • Partial config files should be interpreted and non-specified fields should default to Gopass' default values. I believe this should actually already work with the mostRecent config we've got, but maybe we're missing some default values?

    mostRecent := &Config{
    ExportKeys: true,
    Parsing: true,
    }

    Seems we are missing autoimport, cliptimeout and notifications default values.
    In any case we should make sure to honor the $PASSWORD_STORE_DIR value in case the path is the default one, no?
    I guess the issue in Setting mime:false in config blanks out path, ignores $PASSWORD_STORE_DIR, changes other defaults #1654 is probably that we do not correctly handle an empty path in the config.

  • We should support comments
    Actually I'm surprised that we do not already support comments since it seems we are using a YAML parser, so it should work with the comments as per YAML format, no?

@morningspace
Copy link
Contributor

Just a couple of comments re: the comments idea.

When we say support comments, it looks the implication is that we encourage user to modify the configuration somewhere in a file, so that they can make comments alongside a certain config item. It's intuitive to modify a YAML file and add comments to it.

On the other side, the current gopass config is more like a way that encourage people to change the configuration from the command line which does not need to touch the YAML file directly. If we do not have too many configurable items and people have already been familiar the way as such, I don't see benefits to add the comment support at least for now.

P.S.

Maybe we can take git config as reference, which has both the command and the config file w/ ini-like format. YAML parser should have comment supported. So, if we did not expose the config file format to the end user, this seems to be a nice way that aligns w/ how git user uses git config. To me, it's just not a must have for now.

@dominikschulz
Copy link
Member Author

The git config format is a great idea, but that would be a big change.

@morningspace
Copy link
Contributor

Understood. It may need some sort of tradeoff here. Also, it looks to me the current configurable items for gopass is pretty simple, mostly a small set of flattened key-value pairs, while the YAML format is essentially capable of supporting more complex scenario, including nested structures. This seem to be a bit overkilled.

@dominikschulz
Copy link
Member Author

Yes, it is overkill and I am strongly considering the git config idea.

We have the YAML format because the config used to be more complex (but not complex enough to justify YAML).

@Techassi
Copy link

Just chiming in on this. TOML might be a good config format. The git config format looks to be very close to the TOML format, but it seems like a custom one: Refercence

I would also be open to tackle this migration / change with a PR.

@dominikschulz dominikschulz mentioned this issue Nov 2, 2022
13 tasks
dominikschulz added a commit to dominikschulz/gopass that referenced this issue Nov 6, 2022
This commit adds yet another config handler for gopass. It is based on
the format used by git itself. This has the potential to address a lot
of long standing issues, but it also causes a lot of changes to how we
handle configuration, so bugs are inevitable.

Fixes gopasspw#1567
Fixes gopasspw#1764
Fixes gopasspw#1819
Fixes gopasspw#1878
Fixes gopasspw#2387

RELEASE_NOTES=[BREAKING] New config format based on git config.

Signed-off-by: Dominik Schulz <[email protected]>
dominikschulz added a commit that referenced this issue Nov 9, 2022
This commit adds yet another config handler for gopass. It is based on
the format used by git itself. This has the potential to address a lot
of long standing issues, but it also causes a lot of changes to how we
handle configuration, so bugs are inevitable.

Fixes #1567
Fixes #1764
Fixes #1819
Fixes #1878
Fixes #2387

RELEASE_NOTES=[BREAKING] New config format based on git config.

Signed-off-by: Dominik Schulz <[email protected]>
dominikschulz added a commit to dominikschulz/gopass that referenced this issue Nov 12, 2022
This commit adds yet another config handler for gopass. It is based on
the format used by git itself. This has the potential to address a lot
of long standing issues, but it also causes a lot of changes to how we
handle configuration, so bugs are inevitable.

Fixes gopasspw#1567
Fixes gopasspw#1764
Fixes gopasspw#1819
Fixes gopasspw#1878
Fixes gopasspw#2387

RELEASE_NOTES=[BREAKING] New config format based on git config.

Signed-off-by: Dominik Schulz <[email protected]>
Co-authored-by: Yolan Romailler <[email protected]>
dominikschulz added a commit to dominikschulz/gopass that referenced this issue Nov 17, 2022
This commit adds yet another config handler for gopass. It is based on
the format used by git itself. This has the potential to address a lot
of long standing issues, but it also causes a lot of changes to how we
handle configuration, so bugs are inevitable.

Fixes gopasspw#1567
Fixes gopasspw#1764
Fixes gopasspw#1819
Fixes gopasspw#1878
Fixes gopasspw#2387

RELEASE_NOTES=[BREAKING] New config format based on git config.

Signed-off-by: Dominik Schulz <[email protected]>
Co-authored-by: Yolan Romailler <[email protected]>
dominikschulz added a commit to dominikschulz/gopass that referenced this issue Nov 18, 2022
This commit adds yet another config handler for gopass. It is based on
the format used by git itself. This has the potential to address a lot
of long standing issues, but it also causes a lot of changes to how we
handle configuration, so bugs are inevitable.

Fixes gopasspw#1567
Fixes gopasspw#1764
Fixes gopasspw#1819
Fixes gopasspw#1878
Fixes gopasspw#2387

RELEASE_NOTES=[BREAKING] New config format based on git config.

Signed-off-by: Dominik Schulz <[email protected]>
Co-authored-by: Yolan Romailler <[email protected]>
dominikschulz added a commit to dominikschulz/gopass that referenced this issue Nov 18, 2022
This commit adds yet another config handler for gopass. It is based on
the format used by git itself. This has the potential to address a lot
of long standing issues, but it also causes a lot of changes to how we
handle configuration, so bugs are inevitable.

Fixes gopasspw#1567
Fixes gopasspw#1764
Fixes gopasspw#1819
Fixes gopasspw#1878
Fixes gopasspw#2387

RELEASE_NOTES=[BREAKING] New config format based on git config.

Signed-off-by: Dominik Schulz <[email protected]>
Co-authored-by: Yolan Romailler <[email protected]>
dominikschulz added a commit to dominikschulz/gopass that referenced this issue Nov 18, 2022
This commit adds yet another config handler for gopass. It is based on
the format used by git itself. This has the potential to address a lot
of long standing issues, but it also causes a lot of changes to how we
handle configuration, so bugs are inevitable.

Fixes gopasspw#1567
Fixes gopasspw#1764
Fixes gopasspw#1819
Fixes gopasspw#1878
Fixes gopasspw#2387

RELEASE_NOTES=[BREAKING] New config format based on git config.

Signed-off-by: Dominik Schulz <[email protected]>
Co-authored-by: Yolan Romailler <[email protected]>
dominikschulz added a commit to dominikschulz/gopass that referenced this issue Nov 19, 2022
This commit adds yet another config handler for gopass. It is based on
the format used by git itself. This has the potential to address a lot
of long standing issues, but it also causes a lot of changes to how we
handle configuration, so bugs are inevitable.

Fixes gopasspw#1567
Fixes gopasspw#1764
Fixes gopasspw#1819
Fixes gopasspw#1878
Fixes gopasspw#2387
Fixes gopasspw#2418

RELEASE_NOTES=[BREAKING] New config format based on git config.

Signed-off-by: Dominik Schulz <[email protected]>
Co-authored-by: Yolan Romailler <[email protected]>
dominikschulz added a commit to dominikschulz/gopass that referenced this issue Nov 20, 2022
This commit adds yet another config handler for gopass. It is based on
the format used by git itself. This has the potential to address a lot
of long standing issues, but it also causes a lot of changes to how we
handle configuration, so bugs are inevitable.

Fixes gopasspw#1567
Fixes gopasspw#1764
Fixes gopasspw#1819
Fixes gopasspw#1878
Fixes gopasspw#2387
Fixes gopasspw#2418

RELEASE_NOTES=[BREAKING] New config format based on git config.

Signed-off-by: Dominik Schulz <[email protected]>
Co-authored-by: Yolan Romailler <[email protected]>
dominikschulz added a commit to dominikschulz/gopass that referenced this issue Nov 20, 2022
This commit adds yet another config handler for gopass. It is based on
the format used by git itself. This has the potential to address a lot
of long standing issues, but it also causes a lot of changes to how we
handle configuration, so bugs are inevitable.

Fixes gopasspw#1567
Fixes gopasspw#1764
Fixes gopasspw#1819
Fixes gopasspw#1878
Fixes gopasspw#2387
Fixes gopasspw#2418

RELEASE_NOTES=[BREAKING] New config format based on git config.

Signed-off-by: Dominik Schulz <[email protected]>
Co-authored-by: Yolan Romailler <[email protected]>
dominikschulz added a commit to dominikschulz/gopass that referenced this issue Nov 20, 2022
This commit adds yet another config handler for gopass. It is based on
the format used by git itself. This has the potential to address a lot
of long standing issues, but it also causes a lot of changes to how we
handle configuration, so bugs are inevitable.

Fixes gopasspw#1567
Fixes gopasspw#1764
Fixes gopasspw#1819
Fixes gopasspw#1878
Fixes gopasspw#2387
Fixes gopasspw#2418

RELEASE_NOTES=[BREAKING] New config format based on git config.

Signed-off-by: Dominik Schulz <[email protected]>
Co-authored-by: Yolan Romailler <[email protected]>
dominikschulz added a commit to dominikschulz/gopass that referenced this issue Nov 24, 2022
This commit adds yet another config handler for gopass. It is based on
the format used by git itself. This has the potential to address a lot
of long standing issues, but it also causes a lot of changes to how we
handle configuration, so bugs are inevitable.

Fixes gopasspw#1567
Fixes gopasspw#1764
Fixes gopasspw#1819
Fixes gopasspw#1878
Fixes gopasspw#2387
Fixes gopasspw#2418

RELEASE_NOTES=[BREAKING] New config format based on git config.

Signed-off-by: Dominik Schulz <[email protected]>
Co-authored-by: Yolan Romailler <[email protected]>
dominikschulz added a commit to dominikschulz/gopass that referenced this issue Nov 25, 2022
This commit adds yet another config handler for gopass. It is based on
the format used by git itself. This has the potential to address a lot
of long standing issues, but it also causes a lot of changes to how we
handle configuration, so bugs are inevitable.

Fixes gopasspw#1567
Fixes gopasspw#1764
Fixes gopasspw#1819
Fixes gopasspw#1878
Fixes gopasspw#2387
Fixes gopasspw#2418

RELEASE_NOTES=[BREAKING] New config format based on git config.

Signed-off-by: Dominik Schulz <[email protected]>
Co-authored-by: Yolan Romailler <[email protected]>

address comments

Signed-off-by: Dominik Schulz <[email protected]>
dominikschulz added a commit to dominikschulz/gopass that referenced this issue Nov 25, 2022
This commit adds yet another config handler for gopass. It is based on
the format used by git itself. This has the potential to address a lot
of long standing issues, but it also causes a lot of changes to how we
handle configuration, so bugs are inevitable.

Fixes gopasspw#1567
Fixes gopasspw#1764
Fixes gopasspw#1819
Fixes gopasspw#1878
Fixes gopasspw#2387
Fixes gopasspw#2418

RELEASE_NOTES=[BREAKING] New config format based on git config.

Signed-off-by: Dominik Schulz <[email protected]>
Co-authored-by: Yolan Romailler <[email protected]>

address comments

Signed-off-by: Dominik Schulz <[email protected]>
dominikschulz added a commit to dominikschulz/gopass that referenced this issue Nov 25, 2022
This commit adds yet another config handler for gopass. It is based on
the format used by git itself. This has the potential to address a lot
of long standing issues, but it also causes a lot of changes to how we
handle configuration, so bugs are inevitable.

Fixes gopasspw#1567
Fixes gopasspw#1764
Fixes gopasspw#1819
Fixes gopasspw#1878
Fixes gopasspw#2387
Fixes gopasspw#2418

RELEASE_NOTES=[BREAKING] New config format based on git config.

Signed-off-by: Dominik Schulz <[email protected]>
Co-authored-by: Yolan Romailler <[email protected]>

address comments

Signed-off-by: Dominik Schulz <[email protected]>
dominikschulz added a commit to dominikschulz/gopass that referenced this issue Nov 25, 2022
This commit adds yet another config handler for gopass. It is based on
the format used by git itself. This has the potential to address a lot
of long standing issues, but it also causes a lot of changes to how we
handle configuration, so bugs are inevitable.

Fixes gopasspw#1567
Fixes gopasspw#1764
Fixes gopasspw#1819
Fixes gopasspw#1878
Fixes gopasspw#2387
Fixes gopasspw#2418

RELEASE_NOTES=[BREAKING] New config format based on git config.

Signed-off-by: Dominik Schulz <[email protected]>
Co-authored-by: Yolan Romailler <[email protected]>

address comments

Signed-off-by: Dominik Schulz <[email protected]>
dominikschulz added a commit to dominikschulz/gopass that referenced this issue Nov 25, 2022
This commit adds yet another config handler for gopass. It is based on
the format used by git itself. This has the potential to address a lot
of long standing issues, but it also causes a lot of changes to how we
handle configuration, so bugs are inevitable.

Fixes gopasspw#1567
Fixes gopasspw#1764
Fixes gopasspw#1819
Fixes gopasspw#1878
Fixes gopasspw#2387
Fixes gopasspw#2418

RELEASE_NOTES=[BREAKING] New config format based on git config.

Signed-off-by: Dominik Schulz <[email protected]>
Co-authored-by: Yolan Romailler <[email protected]>

address comments

Signed-off-by: Dominik Schulz <[email protected]>
dominikschulz added a commit that referenced this issue Nov 25, 2022
This commit adds yet another config handler for gopass. It is based on
the format used by git itself. This has the potential to address a lot
of long standing issues, but it also causes a lot of changes to how we
handle configuration, so bugs are inevitable.

Fixes #1567
Fixes #1764
Fixes #1819
Fixes #1878
Fixes #2387
Fixes #2418

RELEASE_NOTES=[BREAKING] New config format based on git config.

Signed-off-by: Dominik Schulz <[email protected]>
Co-authored-by: Yolan Romailler <[email protected]>

address comments

Signed-off-by: Dominik Schulz <[email protected]>

Signed-off-by: Dominik Schulz <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants