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

Remove clientconfig and related global variables #901

Merged
merged 6 commits into from
Aug 4, 2022
Merged

Remove clientconfig and related global variables #901

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
0da7d92
Remove clientconfig and related global variables
Aug 3, 2022
5702ecc
Fix kubeconfig bypassing for unit tests
Aug 3, 2022
d369a88
Fix conflict properly
Aug 4, 2022
b0c17b2
Apply typo fixes
Aug 4, 2022
20a6b78
Fix typo repeated in tests
Aug 4, 2022
db2a0bd
Fix also the internal ref to the param renamed/fixed
Aug 4, 2022
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
72 changes: 45 additions & 27 deletions cmd/kubeseal/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,17 +71,17 @@ var (
reEncrypt bool // re-encrypt command
unseal = flag.Bool("recovery-unseal", false, "Decrypt a sealed secrets file obtained from stdin, using the private key passed with --recovery-private-key. Intended to be used in disaster recovery mode.")
privKeys = flag.StringSlice("recovery-private-key", nil, "Private key filename used by the --recovery-unseal command. Multiple files accepted either via comma separated list or by repetition of the flag. Either PEM encoded private keys or a backup of a json/yaml encoded k8s sealed-secret controller secret (and v1.List) are accepted. ")
kubeconfig = flag.String("kubeconfig", "", "Path to a kube config. Only required if out-of-cluster")

globalOverrides *clientcmd.ConfigOverrides // config overrides from flags

// VERSION set from Makefile
VERSION = buildinfo.DefaultVersion

clientConfig clientcmd.ClientConfig

// testing hook for clientConfig.Namespace()
namespaceFromClientConfig = func() (string, bool, error) { return clientConfig.Namespace() }
)

func init() {
type namespaceFn func() (string, bool, error)

func initFlags() {
alemorcuq marked this conversation as resolved.
Show resolved Hide resolved
buildinfo.FallbackVersion(&VERSION, buildinfo.DefaultVersion)

flag.Var(&sealingScope, "scope", "Set the scope of the sealed secret: strict, namespace-wide, cluster-wide (defaults to strict). Mandatory for --raw, otherwise the 'sealedsecrets.bitnami.com/cluster-wide' and 'sealedsecrets.bitnami.com/namespace-wide' annotations on the input secret can be used to select the scope.")
Expand All @@ -91,14 +91,7 @@ func init() {

flagenv.SetFlagsFromEnv(flagEnvPrefix, goflag.CommandLine)

// The "usual" clientcmd/kubectl flags
loadingRules := clientcmd.NewDefaultClientConfigLoadingRules()
loadingRules.DefaultClientConfig = &clientcmd.DefaultClientConfig
overrides := clientcmd.ConfigOverrides{}
kflags := clientcmd.RecommendedConfigOverrideFlags("")
flag.StringVar(&loadingRules.ExplicitPath, "kubeconfig", "", "Path to a kube config. Only required if out-of-cluster")
clientcmd.BindOverrideFlags(&overrides, flag.CommandLine, kflags)
clientConfig = clientcmd.NewInteractiveDeferredLoadingClientConfig(loadingRules, &overrides, os.Stdin)
globalOverrides = initUsualKubectlFlags(flag.CommandLine)

pflagenv.SetFlagsFromEnv(flagEnvPrefix, flag.CommandLine)

Expand All @@ -108,6 +101,28 @@ func init() {
flag.CommandLine.AddGoFlagSet(goflag.CommandLine)
}

func initUsualKubectlFlags(flagset *flag.FlagSet) *clientcmd.ConfigOverrides {
overrides := clientcmd.ConfigOverrides{}
kflags := clientcmd.RecommendedConfigOverrideFlags("")
clientcmd.BindOverrideFlags(&overrides, flagset, kflags)
return &overrides
}

func initClientFromFlags() clientcmd.ClientConfig {
return initClient(*kubeconfig, *globalOverrides, os.Stdout)
}
josvazg marked this conversation as resolved.
Show resolved Hide resolved
josvazg marked this conversation as resolved.
Show resolved Hide resolved

func initClient(kubeConfigPath string, cfgOverrides clientcmd.ConfigOverrides, w io.Reader) clientcmd.ClientConfig {
josvazg marked this conversation as resolved.
Show resolved Hide resolved
loadingRules := clientcmd.NewDefaultClientConfigLoadingRules()
loadingRules.DefaultClientConfig = &clientcmd.DefaultClientConfig
loadingRules.ExplicitPath = kubeConfigPath
return clientcmd.NewInteractiveDeferredLoadingClientConfig(loadingRules, &cfgOverrides, w)
josvazg marked this conversation as resolved.
Show resolved Hide resolved
}

func initNamespaceFuncFromClient(clientConfig clientcmd.ClientConfig) namespaceFn {
return func() (string, bool, error) { return clientConfig.Namespace() }
}

func parseKey(r io.Reader) (*rsa.PublicKey, error) {
data, err := io.ReadAll(r)
if err != nil {
Expand Down Expand Up @@ -237,7 +252,7 @@ func openCertCluster(ctx context.Context, c corev1.CoreV1Interface, namespace, n
return cert, nil
}

func openCert(ctx context.Context, certURL string) (io.ReadCloser, error) {
func openCert(clientConfig clientcmd.ClientConfig, ctx context.Context, certURL string) (io.ReadCloser, error) {
if certURL != "" {
return openCertLocal(certURL)
}
Expand All @@ -257,7 +272,7 @@ func openCert(ctx context.Context, certURL string) (io.ReadCloser, error) {
// Seal reads a k8s Secret resource parsed from an input reader by a given codec, encrypts all its secrets
// with a given public key, using the name and namespace found in the input secret, unless explicitly overridden
// by the overrideName and overrideNamespace arguments.
func seal(in io.Reader, out io.Writer, codecs runtimeserializer.CodecFactory, pubKey *rsa.PublicKey, scope ssv1alpha1.SealingScope, allowEmptyData bool, overrideName, overrideNamespace string) error {
func seal(in io.Reader, out io.Writer, codecs runtimeserializer.CodecFactory, pubKey *rsa.PublicKey, scope ssv1alpha1.SealingScope, allowEmptyData bool, overrideName, overrideNamespace string, namespaceFromClientConfig namespaceFn) error {
secret, err := readSecret(codecs.UniversalDecoder(), in)
if err != nil {
return err
Expand Down Expand Up @@ -312,7 +327,7 @@ func seal(in io.Reader, out io.Writer, codecs runtimeserializer.CodecFactory, pu
return nil
}

func validateSealedSecret(ctx context.Context, in io.Reader, namespace, name string) error {
func validateSealedSecret(clientConfig clientcmd.ClientConfig, ctx context.Context, in io.Reader, namespace, name string) error {
conf, err := clientConfig.ClientConfig()
if err != nil {
return err
Expand Down Expand Up @@ -350,7 +365,7 @@ func validateSealedSecret(ctx context.Context, in io.Reader, namespace, name str
return nil
}

func reEncryptSealedSecret(ctx context.Context, in io.Reader, out io.Writer, codecs runtimeserializer.CodecFactory, namespace, name string) error {
func reEncryptSealedSecret(clientConfig clientcmd.ClientConfig, ctx context.Context, in io.Reader, out io.Writer, codecs runtimeserializer.CodecFactory, namespace, name string) error {
conf, err := clientConfig.ClientConfig()
if err != nil {
return err
Expand Down Expand Up @@ -433,7 +448,7 @@ func decodeSealedSecret(codecs runtimeserializer.CodecFactory, b []byte) (*ssv1a
return &ss, nil
}

func sealMergingInto(in io.Reader, filename string, codecs runtimeserializer.CodecFactory, pubKey *rsa.PublicKey, scope ssv1alpha1.SealingScope, allowEmptyData bool) error {
func sealMergingInto(in io.Reader, filename string, codecs runtimeserializer.CodecFactory, pubKey *rsa.PublicKey, scope ssv1alpha1.SealingScope, allowEmptyData bool, namespaceFromClientConfig namespaceFn) error {
// #nosec G304 -- should open user provided file
f, err := os.OpenFile(filename, os.O_RDWR, 0)
if err != nil {
Expand All @@ -453,7 +468,7 @@ func sealMergingInto(in io.Reader, filename string, codecs runtimeserializer.Cod
}

var buf bytes.Buffer
if err := seal(in, &buf, codecs, pubKey, scope, allowEmptyData, orig.Name, orig.Namespace); err != nil {
if err := seal(in, &buf, codecs, pubKey, scope, allowEmptyData, orig.Name, orig.Namespace, namespaceFromClientConfig); err != nil {
return err
}

Expand Down Expand Up @@ -632,7 +647,7 @@ func unsealSealedSecret(w io.Writer, in io.Reader, codecs runtimeserializer.Code
return resourceOutput(w, codecs, v1.SchemeGroupVersion, sec)
}

func run(ctx context.Context, w io.Writer, inputFileName, outputFileName, secretName, controllerNs, controllerName, certURL string, printVersion, validateSecret, reEncrypt, dumpCert, raw, allowEmptyData bool, fromFile []string, mergeInto string, unseal bool, privKeys []string) (err error) {
func run(clientConfig clientcmd.ClientConfig, ctx context.Context, w io.Writer, inputFileName, outputFileName, secretName, controllerNs, controllerName, certURL string, printVersion, validateSecret, reEncrypt, dumpCert, raw, allowEmptyData bool, fromFile []string, mergeInto string, unseal bool, privKeys []string, namespaceFromClientConfig namespaceFn) (err error) {
if len(fromFile) != 0 && !raw {
return fmt.Errorf("--from-file requires --raw")
}
Expand Down Expand Up @@ -693,14 +708,14 @@ func run(ctx context.Context, w io.Writer, inputFileName, outputFileName, secret
}

if validateSecret {
return validateSealedSecret(ctx, input, controllerNs, controllerName)
return validateSealedSecret(clientConfig, ctx, input, controllerNs, controllerName)
}

if reEncrypt {
return reEncryptSealedSecret(ctx, input, w, scheme.Codecs, controllerNs, controllerName)
return reEncryptSealedSecret(clientConfig, ctx, input, w, scheme.Codecs, controllerNs, controllerName)
}

f, err := openCert(ctx, certURL)
f, err := openCert(clientConfig, ctx, certURL)
if err != nil {
return err
}
Expand All @@ -717,7 +732,7 @@ func run(ctx context.Context, w io.Writer, inputFileName, outputFileName, secret
}

if mergeInto != "" {
return sealMergingInto(input, mergeInto, scheme.Codecs, pubKey, sealingScope, allowEmptyData)
return sealMergingInto(input, mergeInto, scheme.Codecs, pubKey, sealingScope, allowEmptyData, namespaceFromClientConfig)
}

if raw {
Expand Down Expand Up @@ -762,14 +777,17 @@ func run(ctx context.Context, w io.Writer, inputFileName, outputFileName, secret
return encryptSecretItem(w, secretName, ns, data, sealingScope, pubKey)
}

return seal(input, w, scheme.Codecs, pubKey, sealingScope, allowEmptyData, secretName, "")
return seal(input, w, scheme.Codecs, pubKey, sealingScope, allowEmptyData, secretName, "", namespaceFromClientConfig)
}

func main() {
initFlags()
flag.Parse()
_ = goflag.CommandLine.Parse([]string{})

if err := run(context.Background(), os.Stdout, *inputFileName, *outputFileName, *secretName, *controllerNs, *controllerName, *certURL, *printVersion, *validateSecret, reEncrypt, *dumpCert, *raw, *allowEmptyData, *fromFile, *mergeInto, *unseal, *privKeys); err != nil {
clientConfig := initClientFromFlags()
namespaceFromClientConfig := initNamespaceFuncFromClient(clientConfig)
if err := run(clientConfig, context.Background(), os.Stdout, *inputFileName, *outputFileName, *secretName, *controllerNs, *controllerName, *certURL, *printVersion, *validateSecret, reEncrypt, *dumpCert, *raw, *allowEmptyData, *fromFile, *mergeInto, *unseal, *privKeys, namespaceFromClientConfig); err != nil {
fmt.Fprintf(os.Stderr, "error: %v\n", err)
os.Exit(1)
}
Expand Down
75 changes: 40 additions & 35 deletions cmd/kubeseal/main_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (
"crypto/rsa"
"crypto/x509"
"encoding/pem"
goflag "flag"
"fmt"
"io"
"math/big"
Expand All @@ -20,13 +19,15 @@ import (
"testing"
"time"

flag "github.com/spf13/pflag"

"github.com/google/go-cmp/cmp"
"github.com/google/go-cmp/cmp/cmpopts"
flag "github.com/spf13/pflag"
v1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/client-go/kubernetes/scheme"
"k8s.io/client-go/tools/clientcmd"
certUtil "k8s.io/client-go/util/cert"
"k8s.io/client-go/util/keyutil"

Expand Down Expand Up @@ -78,16 +79,6 @@ func init() {
}
}

func TestMain(m *testing.M) {
// ensure that the -test.* flags inserted by go test are properly processed
// otherwise the pflag.Parse invocation below will interfere with test flags.
goflag.Parse()

// otherwise we'd require a working KUBECONFIG file when calling `run`.
_ = flag.CommandLine.Parse([]string{"-n", "default"})
os.Exit(m.Run())
}

func tmpfile(t *testing.T, contents []byte) string {
f, err := os.CreateTemp("", "testdata")
if err != nil {
Expand All @@ -102,6 +93,21 @@ func tmpfile(t *testing.T, contents []byte) string {
return f.Name()
}

func testConfigOverrides() *clientcmd.ConfigOverrides {
flagset := flag.NewFlagSet("test", flag.PanicOnError)
testOverrides := initUsualKubectlFlags(flagset)
err := flagset.Parse([]string{"-n", "default"})
if err != nil {
fmt.Printf("flagset parse err: %v\n", err)
os.Exit(1)
}
return testOverrides
}

func testClientConfig() clientcmd.ClientConfig {
return initClient("", *testConfigOverrides(), os.Stdout)
josvazg marked this conversation as resolved.
Show resolved Hide resolved
}

func TestParseKey(t *testing.T) {
key, err := parseKey(strings.NewReader(testCert))
if err != nil {
Expand Down Expand Up @@ -135,7 +141,7 @@ func TestOpenCertFile(t *testing.T) {
}

for _, certURL := range testCases {
f, err := openCert(ctx, certURL)
f, err := openCert(testClientConfig(), ctx, certURL)
if err != nil {
t.Fatalf("Error reading test cert file: %v", err)
}
Expand Down Expand Up @@ -325,8 +331,9 @@ func TestSeal(t *testing.T) {

t.Logf("input is: %s", inbuf.String())

namespaceFromClientConfig := initNamespaceFuncFromClient(testClientConfig())
outbuf := bytes.Buffer{}
if err := seal(&inbuf, &outbuf, scheme.Codecs, key, tc.scope, false, "", ""); err != nil {
if err := seal(&inbuf, &outbuf, scheme.Codecs, key, tc.scope, false, "", "", namespaceFromClientConfig); err != nil {
t.Fatalf("seal() returned error: %v", err)
}

Expand Down Expand Up @@ -433,8 +440,9 @@ func mkTestSecret(t *testing.T, key, value string, opts ...mkTestSecretOpt) []by

func mkTestSealedSecret(t *testing.T, pubKey *rsa.PublicKey, key, value string, opts ...mkTestSecretOpt) []byte {
inbuf := bytes.NewBuffer(mkTestSecret(t, key, value, opts...))
namespaceFromClientConfig := initNamespaceFuncFromClient(testClientConfig())
var outbuf bytes.Buffer
if err := seal(inbuf, &outbuf, scheme.Codecs, pubKey, ssv1alpha1.DefaultScope, false, "", ""); err != nil {
if err := seal(inbuf, &outbuf, scheme.Codecs, pubKey, ssv1alpha1.DefaultScope, false, "", "", namespaceFromClientConfig); err != nil {
t.Fatalf("seal() returned error: %v", err)
}

Expand Down Expand Up @@ -576,8 +584,9 @@ func TestMergeInto(t *testing.T) {
}
f.Close()

namespaceFromClientConfig := initNamespaceFuncFromClient(testClientConfig())
buf := bytes.NewBuffer(newSecret)
if err := sealMergingInto(buf, f.Name(), scheme.Codecs, pubKey, ssv1alpha1.DefaultScope, false); err != nil {
if err := sealMergingInto(buf, f.Name(), scheme.Codecs, pubKey, ssv1alpha1.DefaultScope, false, namespaceFromClientConfig); err != nil {
t.Fatal(err)
}

Expand Down Expand Up @@ -665,7 +674,9 @@ func TestMergeInto(t *testing.T) {
func TestVersion(t *testing.T) {
ctx := context.Background()
var buf strings.Builder
err := run(ctx, &buf, "", "", "", "", "", "", true, false, false, false, false, false, nil, "", false, nil)
testClient := testClientConfig()
namespaceFromClientConfig := initNamespaceFuncFromClient(testClient)
err := run(testClient, ctx, &buf, "", "", "", "", "", "", true, false, false, false, false, false, nil, "", false, nil, namespaceFromClientConfig)
if err != nil {
t.Fatal(err)
}
Expand All @@ -678,7 +689,9 @@ func TestVersion(t *testing.T) {
func TestMainError(t *testing.T) {
ctx := context.Background()
badFileName := filepath.Join("this", "file", "cannot", "possibly", "exist", "can", "it?")
err := run(ctx, io.Discard, "", "", "", "", "", badFileName, false, false, false, false, false, false, nil, "", false, nil)
testClient := testClientConfig()
namespaceFromClientConfig := initNamespaceFuncFromClient(testClient)
err := run(testClient, ctx, io.Discard, "", "", "", "", "", badFileName, false, false, false, false, false, false, nil, "", false, nil, namespaceFromClientConfig)

if err == nil || !os.IsNotExist(err) {
t.Fatalf("expecting not exist error, got: %v", err)
Expand Down Expand Up @@ -812,24 +825,12 @@ func TestRaw(t *testing.T) {
}

func sealTestItem(certFilename, secretNS, secretName, secretValue string, scope ssv1alpha1.SealingScope) (string, error) {
// we use a global k8s config from which we take the namespace (either default or set by flag).
// it's a mess, for now let's hook in a test getter and restore the original getter after the test.
defer func(s func() (string, bool, error)) { namespaceFromClientConfig = s }(namespaceFromClientConfig)
namespaceFromClientConfig = func() (string, bool, error) { return secretNS, false, nil }
namespaceFromClientConfig := func() (string, bool, error) { return secretNS, false, nil }

// sadly, sealingscope is also global
// sadly, sealingscope is global
// TODO(mkm): refactor this mess
defer func(s ssv1alpha1.SealingScope) { sealingScope = s }(sealingScope)
sealingScope = scope
/*
if got, want := run(io.Discard, "", "", "", certFilename, false, false, false, false, true, nil, "", false, nil), "must provide the --name flag with --raw and --scope strict"; got == nil || got.Error() != want {
t.Fatalf("want matching: %q, got: %q", want, got.Error())
}

if got, want := run(io.Discard, secretName, "", "", certFilename, false, false, false, false, true, nil, "", false, nil), "must provide the --from-file flag with --raw"; got == nil || got.Error() != want {
t.Fatalf("want matching: %q, got: %q", want, got.Error())
}
*/

dataFile, err := writeTempFile([]byte(secretValue))
if err != nil {
Expand All @@ -841,7 +842,7 @@ func sealTestItem(certFilename, secretNS, secretName, secretValue string, scope

ctx := context.Background()
var buf bytes.Buffer
if err := run(ctx, &buf, "", "", secretName, "", "", certFilename, false, false, false, false, true, false, fromFile, "", false, nil); err != nil {
if err := run(testClientConfig(), ctx, &buf, "", "", secretName, "", "", certFilename, false, false, false, false, true, false, fromFile, "", false, nil, namespaceFromClientConfig); err != nil {
return "", err
}

Expand Down Expand Up @@ -876,7 +877,9 @@ data:

ctx := context.Background()
var buf bytes.Buffer
if err := run(ctx, &buf, in.Name(), out.Name(), "", "", "", certFilename, false, false, false, false, false, false, nil, "", false, nil); err != nil {
testClient := testClientConfig()
namespaceFromClientConfig := initNamespaceFuncFromClient(testClient)
if err := run(testClient, ctx, &buf, in.Name(), out.Name(), "", "", "", certFilename, false, false, false, false, false, false, nil, "", false, nil, namespaceFromClientConfig); err != nil {
t.Fatal(err)
}

Expand Down Expand Up @@ -924,7 +927,9 @@ metadata:

ctx := context.Background()
var buf bytes.Buffer
if err := run(ctx, &buf, in.Name(), out.Name(), "", "", "", certFilename, false, false, false, false, false, false, nil, "", false, nil); err == nil {
testClient := testClientConfig()
namespaceFromClientConfig := initNamespaceFuncFromClient(testClient)
if err := run(testClient, ctx, &buf, in.Name(), out.Name(), "", "", "", certFilename, false, false, false, false, false, false, nil, "", false, nil, namespaceFromClientConfig); err == nil {
t.Errorf("expecting error")
}

Expand Down