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

age: improve identity loading, add tests, tidy #1064

Merged
merged 1 commit into from
Jun 6, 2022
Merged

age: improve identity loading, add tests, tidy #1064

merged 1 commit into from
Jun 6, 2022

Conversation

hiddeco
Copy link
Member

@hiddeco hiddeco commented May 27, 2022

This adds improvements to identity loading, extensive test coverage
and a general tidying of bits of code. The improvements are based on a
fork of the age key source in the Flux project's kustomize-controller,
which was built due to SOPS' limitations around identity management
without relying on runtime environment variables.

  • It introduces a ParsedIdentity type which contains a slice of age
    identities, and can be applied to the MasterKey. When applied,
    further loading of identities from the runtime environment is skipped
    for Decrypt operations. This is most useful when working with SOPS
    as an SDK, in combination with e.g. a local key service server
    implementation.
  • The Identity field has been deprecated in the MasterKey struct.
    Presence of the field was misleading, as it is not actually used.
  • Any detected identity reference is now loaded, instead of it assuming
    a priority order. This makes more sense, as age is able to work with
    a set of loaded identities. If no environment variables are defined,
    the existence of the keys.txt in the user's config directory is
    required.
  • Decrypt logs have been added to match other key sources.
  • Extensive test coverage.

if err != nil {
return nil, err
return nil, fmt.Errorf("failed to open %s file: %w", SopsAgeKeyFileEnv, err)
Copy link
Member Author

Choose a reason for hiding this comment

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

The other option would be to log any loading failure, and only error if identities == 0.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this way is preferable. A bit more defense around user mistakes, especially in automation using sops.

@hiddeco hiddeco mentioned this pull request Jun 2, 2022
This adds improvements to identity loading, extensive test coverage
and a general tidying of bits of code. The improvements are based on a
fork of the age key source in the Flux project's kustomize-controller,
which was built due to SOPS' limitations around identity management
without relying on runtime environment variables.

- It introduces a `ParsedIdentity` type which contains a slice of age
  identities, and can be applied to the `MasterKey`. When applied,
  further loading of identities from the runtime environment is skipped
  for `Decrypt` operations. This is most useful when working with SOPS
  as an SDK, in combination with e.g. a local key service server
  implementation.
- The `Identity` field has been deprecated in the `MasterKey` struct.
  Presence of the field was misleading, as it is not actually used.
- Any detected identity reference is now loaded, instead of it assuming
  a priority order. This makes more sense, as age is able to work with
  a set of loaded identities. If no environment variables are defined,
  the existence of the keys.txt in the user's config directory is
  required.
- Decrypt logs have been added to match other key sources.
- Extensive test coverage.

Signed-off-by: Hidde Beydals <hello@hidde.co>
if err != nil {
return nil, err
return nil, fmt.Errorf("failed to open %s file: %w", SopsAgeKeyFileEnv, err)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this way is preferable. A bit more defense around user mistakes, especially in automation using sops.

@ajvb
Copy link
Contributor

ajvb commented Jun 6, 2022

This is awesome, thank you @hiddeco!

@ajvb ajvb merged commit f49164a into getsops:develop Jun 6, 2022
@hiddeco hiddeco deleted the age-keysource-imprv branch June 7, 2022 19:46
@hiddeco hiddeco added this to the v3.8.0 milestone Jul 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants