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

azkv: update SDK to latest, add tests, tidy #1067

Merged
merged 3 commits into from
Jun 6, 2022
Merged

azkv: update SDK to latest, add tests, tidy #1067

merged 3 commits into from
Jun 6, 2022

Conversation

hiddeco
Copy link
Member

@hiddeco hiddeco commented May 30, 2022

This updates the Azure SDK to latest, while dropping the custom
authentication flow in favor of the SDK default. It includes
integration tests, which require the integration Go build tag and
a set of environmental variables to be configured to be run:

PASS
coverage: 81.2% of statements
ok  	go.mozilla.org/sops/v3/azkv	5.376s	coverage: 81.2% of statements

The improvements are based on a fork of the key source in the Flux
project's kustomize-controller, built due to SOPS' limitation around
credential managment without relying on runtime environment variables.

  • Azure SDK has been updated to latest, including integration test
    coverage.
  • Custom authentication flow has been dropped in favor of the SDK
    default
    . This should work well on almost any system and is
    generally the go-to way of setting this up, including on cloud
    environments, etc.
  • It introduces a TokenCredential type which holds an
    azcore.TokenCredential, and can be applied to a MasterKey.
    When applied, the token is used instead of the SDK default. This is
    most useful when working with SOPS as an SDK, in combination with
    e.g. a local key service server implementation.
  • Extensive test coverage.

The forked version of this has compatibility tests to ensure it works
with current SOPS:

@hiddeco hiddeco marked this pull request as ready for review May 30, 2022 13:49
@hiddeco hiddeco changed the title azkv: update to latest client, add tests, tidy azkv: update SDK to latest, add tests, tidy May 30, 2022
@hiddeco hiddeco requested a review from ajvb May 30, 2022 14:18
Copy link
Contributor

@ajvb ajvb left a comment

Choose a reason for hiding this comment

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

LGTM! Just need to update the README and we'll be ready to merge.

azkv/keysource.go Show resolved Hide resolved
pgp/keysource_test.go Outdated Show resolved Hide resolved
This is required for the latest Azure SDK, and comes with general
improvements for certain CPU types.

Includes a change of `%w` -> `%v` for `t.Errorf` due to dropped support
for wrapping.

Signed-off-by: Hidde Beydals <[email protected]>
This updates the Azure SDK to latest[1], while dropping the custom
authentication flow in favor of the SDK default[2]. It includes
integration tests, which require the `integration` Go build tag and
a set of environmental variables to be configured to be run:

```
PASS
coverage: 81.2% of statements
ok  	go.mozilla.org/sops/v3/azkv	5.376s	coverage: 81.2% of statements
```

The improvements are based on a fork of the key source in the Flux
project's kustomize-controller, built due to SOPS' limitation around
credential managment without relying on runtime environment variables.

- Azure SDK has been updated to latest, including integration test
  coverage.
- Custom authentication flow has been dropped in favor of the SDK
  default[2]. This should work well on almost any system and is
  generally the go-to way of setting this up, including on cloud
  environments, etc.
- It introduces a `TokenCredential` type which holds an
  `azcore.TokenCredential`, and can be applied to a `MasterKey`.
  When applied, the token is used instead of the SDK default. This is
  most useful when working with SOPS as an SDK, in combination with
  e.g. a local key service server implementation.
- Extensive test coverage.

The forked version of this has compatibility tests to ensure it works
with current SOPS:

- https://github.com/fluxcd/kustomize-controller/blob/327a3560b35c5994647938958aa5523f319094b8/internal/sops/azkv/keysource_integration_test.go#L89
- https://github.com/fluxcd/kustomize-controller/blob/327a3560b35c5994647938958aa5523f319094b8/internal/sops/azkv/keysource_integration_test.go#L117

[1]: https://github.com/Azure/azure-sdk-for-go/tree/main/sdk/keyvault/azkeys
[2]: https://pkg.go.dev/github.com/Azure/azure-sdk-for-go/sdk/azidentity#DefaultAzureCredential

Signed-off-by: Hidde Beydals <[email protected]>
Plus update mention of Go version requirement.

Signed-off-by: Hidde Beydals <[email protected]>
Copy link
Contributor

@ajvb ajvb left a comment

Choose a reason for hiding this comment

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

🎉

@ajvb ajvb merged commit 7783c1e into getsops:develop Jun 6, 2022
@hiddeco hiddeco deleted the azkv-keysource-imprv branch June 7, 2022 19:47
@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