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.

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

Jump to bottom

fix(config): allow usage of full config struct #1683

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

fix(config): allow usage of full config struct #1683

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
94 changes: 43 additions & 51 deletions config/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,19 +17,11 @@ import (
"github.com/getsops/sops/v3/gcpkms"
"github.com/getsops/sops/v3/hcvault"
"github.com/getsops/sops/v3/kms"
"github.com/getsops/sops/v3/logging"
"github.com/getsops/sops/v3/pgp"
"github.com/getsops/sops/v3/publish"
"github.com/sirupsen/logrus"
"gopkg.in/yaml.v3"
)

var log *logrus.Logger

func init() {
log = logging.NewLogger("CONFIG")
}
Comment on lines -27 to -31
Copy link
Author

Choose a reason for hiding this comment

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

I got a warning from go-staticcheck and I could not see this being used anywhere so I removed it to avoid the warning.

Copy link
Contributor

Choose a reason for hiding this comment

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

Better move the unrelated changes to a separate PR. That will increase the chance that these get merged, and make reviewing simpler generally.

Regarding this specific change: log and the init() function were added in d3d0267. The last code using log was removed in ebd153f. So there's definitely no reason to keep this.


type fileSystem interface {
Stat(name string) (os.FileInfo, error)
}
Expand Down Expand Up @@ -87,40 +79,41 @@ type StoresConfig struct {
YAML YAMLStoreConfig `yaml:"yaml"`
}

type configFile struct {
CreationRules []creationRule `yaml:"creation_rules"`
DestinationRules []destinationRule `yaml:"destination_rules"`
// ConfigFile is the struct representation of a full sops config file
type ConfigFile struct {
CreationRules []CreationRule `yaml:"creation_rules"`
DestinationRules []CestinationRule `yaml:"destination_rules"`
Stores StoresConfig `yaml:"stores"`
}

type keyGroup struct {
Merge []keyGroup
KMS []kmsKey
GCPKMS []gcpKmsKey `yaml:"gcp_kms"`
AzureKV []azureKVKey `yaml:"azure_keyvault"`
type KeyGroup struct {
Merge []KeyGroup
KMS []KmsKey
GCPKMS []GcpKmsKey `yaml:"gcp_kms"`
AzureKV []AzureKVKey `yaml:"azure_keyvault"`
Vault []string `yaml:"hc_vault"`
Age []string `yaml:"age"`
PGP []string
}

type gcpKmsKey struct {
type GcpKmsKey struct {
ResourceID string `yaml:"resource_id"`
}

type kmsKey struct {
type KmsKey struct {
Arn string `yaml:"arn"`
Role string `yaml:"role,omitempty"`
Context map[string]*string `yaml:"context"`
AwsProfile string `yaml:"aws_profile"`
}

type azureKVKey struct {
type AzureKVKey struct {
VaultURL string `yaml:"vaultUrl"`
Key string `yaml:"key"`
Version string `yaml:"version"`
}

type destinationRule struct {
type CestinationRule struct {
PathRegex string `yaml:"path_regex"`
S3Bucket string `yaml:"s3_bucket"`
S3Prefix string `yaml:"s3_prefix"`
Expand All @@ -130,11 +123,11 @@ type destinationRule struct {
VaultAddress string `yaml:"vault_address"`
VaultKVMountName string `yaml:"vault_kv_mount_name"`
VaultKVVersion int `yaml:"vault_kv_version"`
RecreationRule creationRule `yaml:"recreation_rule,omitempty"`
RecreationRule CreationRule `yaml:"recreation_rule,omitempty"`
OmitExtensions bool `yaml:"omit_extensions"`
}

type creationRule struct {
type CreationRule struct {
PathRegex string `yaml:"path_regex"`
KMS string
AwsProfile string `yaml:"aws_profile"`
Expand All @@ -143,7 +136,7 @@ type creationRule struct {
GCPKMS string `yaml:"gcp_kms"`
AzureKeyVault string `yaml:"azure_keyvault"`
VaultURI string `yaml:"hc_vault_transit_uri"`
KeyGroups []keyGroup `yaml:"key_groups"`
KeyGroups []KeyGroup `yaml:"key_groups"`
ShamirThreshold int `yaml:"shamir_threshold"`
UnencryptedSuffix string `yaml:"unencrypted_suffix"`
EncryptedSuffix string `yaml:"encrypted_suffix"`
Expand All @@ -162,10 +155,10 @@ func NewStoresConfig() *StoresConfig {
}

// Load loads a sops config file into a temporary struct
func (f *configFile) load(bytes []byte) error {
func (f *ConfigFile) load(bytes []byte) error {
err := yaml.Unmarshal(bytes, f)
if err != nil {
return fmt.Errorf("Could not unmarshal config file: %s", err)
return fmt.Errorf("could not unmarshal config file: %s", err)
Copy link
Author

Choose a reason for hiding this comment

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

This was also flagged by go-staticcheck.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is a change that should go into a feature release and not into a bugfix release since it changes the observable behavior a bit and isn't a bugfix. Because of that I would keep it separate from the other changes.

}
return nil
}
Expand Down Expand Up @@ -203,7 +196,7 @@ func deduplicateKeygroup(group sops.KeyGroup) sops.KeyGroup {
return deduplicatedKeygroup
}

func extractMasterKeys(group keyGroup) (sops.KeyGroup, error) {
func extractMasterKeys(group KeyGroup) (sops.KeyGroup, error) {
var keyGroup sops.KeyGroup
for _, k := range group.Merge {
subKeyGroup, err := extractMasterKeys(k)
Expand Down Expand Up @@ -244,7 +237,7 @@ func extractMasterKeys(group keyGroup) (sops.KeyGroup, error) {
return deduplicateKeygroup(keyGroup), nil
}

func getKeyGroupsFromCreationRule(cRule *creationRule, kmsEncryptionContext map[string]*string) ([]sops.KeyGroup, error) {
func getKeyGroupsFromCreationRule(cRule *CreationRule, kmsEncryptionContext map[string]*string) ([]sops.KeyGroup, error) {
var groups []sops.KeyGroup
if len(cRule.KeyGroups) > 0 {
for _, group := range cRule.KeyGroups {
Expand Down Expand Up @@ -294,12 +287,13 @@ func getKeyGroupsFromCreationRule(cRule *creationRule, kmsEncryptionContext map[
return groups, nil
}

func loadConfigFile(confPath string) (*configFile, error) {
// LoadConfigFile loads a sops config file from a given path
func LoadConfigFile(confPath string) (*ConfigFile, error) {
confBytes, err := os.ReadFile(confPath)
if err != nil {
return nil, fmt.Errorf("could not read config file: %s", err)
}
conf := &configFile{}
conf := &ConfigFile{}
conf.Stores = *NewStoresConfig()
err = conf.load(confBytes)
if err != nil {
Expand All @@ -308,7 +302,7 @@ func loadConfigFile(confPath string) (*configFile, error) {
return conf, nil
}

func configFromRule(rule *creationRule, kmsEncryptionContext map[string]*string) (*Config, error) {
func configFromRule(rule *CreationRule, kmsEncryptionContext map[string]*string) (*Config, error) {
cryptRuleCount := 0
if rule.UnencryptedSuffix != "" {
cryptRuleCount++
Expand Down Expand Up @@ -351,9 +345,9 @@ func configFromRule(rule *creationRule, kmsEncryptionContext map[string]*string)
}, nil
}

func parseDestinationRuleForFile(conf *configFile, filePath string, kmsEncryptionContext map[string]*string) (*Config, error) {
var rule *creationRule
var dRule *destinationRule
func parseDestinationRuleForFile(conf *ConfigFile, filePath string, kmsEncryptionContext map[string]*string) (*Config, error) {
var rule *CreationRule
var dRule *CestinationRule

if len(conf.DestinationRules) > 0 {
for _, r := range conf.DestinationRules {
Expand All @@ -377,19 +371,17 @@ func parseDestinationRuleForFile(conf *configFile, filePath string, kmsEncryptio
}

var dest publish.Destination
if dRule != nil {
if dRule.S3Bucket != "" && dRule.GCSBucket != "" && dRule.VaultPath != "" {
return nil, fmt.Errorf("error loading config: more than one destinations were found in a single destination rule, you can only use one per rule")
}
if dRule.S3Bucket != "" {
dest = publish.NewS3Destination(dRule.S3Bucket, dRule.S3Prefix)
}
if dRule.GCSBucket != "" {
dest = publish.NewGCSDestination(dRule.GCSBucket, dRule.GCSPrefix)
}
if dRule.VaultPath != "" {
dest = publish.NewVaultDestination(dRule.VaultAddress, dRule.VaultPath, dRule.VaultKVMountName, dRule.VaultKVVersion)
}
Comment on lines -380 to -392
Copy link
Author

Choose a reason for hiding this comment

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

This was flagged as tautological condition: non-nil != nil by nilness.

if dRule.S3Bucket != "" && dRule.GCSBucket != "" && dRule.VaultPath != "" {
return nil, fmt.Errorf("error loading config: more than one destinations were found in a single destination rule, you can only use one per rule")
}
if dRule.S3Bucket != "" {
dest = publish.NewS3Destination(dRule.S3Bucket, dRule.S3Prefix)
}
if dRule.GCSBucket != "" {
dest = publish.NewGCSDestination(dRule.GCSBucket, dRule.GCSPrefix)
}
if dRule.VaultPath != "" {
dest = publish.NewVaultDestination(dRule.VaultAddress, dRule.VaultPath, dRule.VaultKVMountName, dRule.VaultKVVersion)
}

config, err := configFromRule(rule, kmsEncryptionContext)
Expand All @@ -402,7 +394,7 @@ func parseDestinationRuleForFile(conf *configFile, filePath string, kmsEncryptio
return config, nil
}

func parseCreationRuleForFile(conf *configFile, confPath, filePath string, kmsEncryptionContext map[string]*string) (*Config, error) {
func parseCreationRuleForFile(conf *ConfigFile, confPath, filePath string, kmsEncryptionContext map[string]*string) (*Config, error) {
// If config file doesn't contain CreationRules (it's empty or only contains DestionationRules), assume it does not exist
if conf.CreationRules == nil {
return nil, nil
Expand All @@ -416,7 +408,7 @@ func parseCreationRuleForFile(conf *configFile, confPath, filePath string, kmsEn
// compare file path relative to path of config file
filePath = strings.TrimPrefix(filePath, configDir+string(filepath.Separator))

var rule *creationRule
var rule *CreationRule

for _, r := range conf.CreationRules {
if r.PathRegex == "" {
Expand Down Expand Up @@ -449,7 +441,7 @@ func parseCreationRuleForFile(conf *configFile, confPath, filePath string, kmsEn
// should be provided for configurations that do not contain key groups, as there's no way to specify context inside
// a SOPS config file outside of key groups.
func LoadCreationRuleForFile(confPath string, filePath string, kmsEncryptionContext map[string]*string) (*Config, error) {
conf, err := loadConfigFile(confPath)
conf, err := LoadConfigFile(confPath)
if err != nil {
return nil, err
}
Expand All @@ -460,15 +452,15 @@ func LoadCreationRuleForFile(confPath string, filePath string, kmsEncryptionCont
// LoadDestinationRuleForFile works the same as LoadCreationRuleForFile, but gets the "creation_rule" from the matching destination_rule's
// "recreation_rule".
func LoadDestinationRuleForFile(confPath string, filePath string, kmsEncryptionContext map[string]*string) (*Config, error) {
conf, err := loadConfigFile(confPath)
conf, err := LoadConfigFile(confPath)
if err != nil {
return nil, err
}
return parseDestinationRuleForFile(conf, filePath, kmsEncryptionContext)
}

func LoadStoresConfig(confPath string) (*StoresConfig, error) {
conf, err := loadConfigFile(confPath)
conf, err := LoadConfigFile(confPath)
if err != nil {
return nil, err
}
Expand Down
30 changes: 15 additions & 15 deletions config/config_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -387,16 +387,16 @@ creation_rules:
kms: default
`)

func parseConfigFile(confBytes []byte, t *testing.T) *configFile {
conf := &configFile{}
func parseConfigFile(confBytes []byte, t *testing.T) *ConfigFile {
conf := &ConfigFile{}
err := conf.load(confBytes)
assert.Nil(t, err)
return conf
}

func TestLoadConfigFile(t *testing.T) {
expected := configFile{
CreationRules: []creationRule{
expected := ConfigFile{
CreationRules: []CreationRule{
{
PathRegex: "foobar*",
KMS: "1",
Expand All @@ -414,46 +414,46 @@ func TestLoadConfigFile(t *testing.T) {
},
}

conf := configFile{}
conf := ConfigFile{}
err := conf.load(sampleConfig)
assert.Nil(t, err)
assert.Equal(t, expected, conf)
}

func TestLoadConfigFileWithGroups(t *testing.T) {
expected := configFile{
CreationRules: []creationRule{
expected := ConfigFile{
CreationRules: []CreationRule{
{
PathRegex: "foobar*",
KMS: "1",
PGP: "2",
},
{
PathRegex: "",
KeyGroups: []keyGroup{
KeyGroups: []KeyGroup{
{
KMS: []kmsKey{{Arn: "foo", AwsProfile: "bar"}},
KMS: []KmsKey{{Arn: "foo", AwsProfile: "bar"}},
PGP: []string{"bar"},
GCPKMS: []gcpKmsKey{{ResourceID: "foo"}},
AzureKV: []azureKVKey{{VaultURL: "https://foo.vault.azure.net", Key: "foo-key", Version: "fooversion"}},
GCPKMS: []GcpKmsKey{{ResourceID: "foo"}},
AzureKV: []AzureKVKey{{VaultURL: "https://foo.vault.azure.net", Key: "foo-key", Version: "fooversion"}},
Vault: []string{"https://foo.vault:8200/v1/foo/keys/foo-key"},
},
{
KMS: []kmsKey{{Arn: "baz", AwsProfile: "foo"}},
KMS: []KmsKey{{Arn: "baz", AwsProfile: "foo"}},
PGP: []string{"qux"},
GCPKMS: []gcpKmsKey{
GCPKMS: []GcpKmsKey{
{ResourceID: "bar"},
{ResourceID: "baz"},
},
AzureKV: []azureKVKey{{VaultURL: "https://bar.vault.azure.net", Key: "bar-key", Version: "barversion"}},
AzureKV: []AzureKVKey{{VaultURL: "https://bar.vault.azure.net", Key: "bar-key", Version: "barversion"}},
Vault: []string{"https://baz.vault:8200/v1/baz/keys/baz-key"},
},
},
},
},
}

conf := configFile{}
conf := ConfigFile{}
err := conf.load(sampleConfigWithGroups)
assert.Nil(t, err)
assert.Equal(t, expected, conf)
Expand Down