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

fix: address some golangci-lint issues #751

Merged
merged 2 commits into from
Mar 3, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
98 changes: 19 additions & 79 deletions cmd/controller/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import (
"k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
runtimeserializer "k8s.io/apimachinery/pkg/runtime/serializer"
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
"k8s.io/apimachinery/pkg/util/wait"
"k8s.io/client-go/kubernetes"
Expand All @@ -25,7 +24,6 @@ import (
"k8s.io/klog"

"github.com/bitnami-labs/sealed-secrets/pkg/apis/sealed-secrets/v1alpha1"
ssv1alpha1 "github.com/bitnami-labs/sealed-secrets/pkg/apis/sealed-secrets/v1alpha1"
juan131 marked this conversation as resolved.
Show resolved Hide resolved
ssclientset "github.com/bitnami-labs/sealed-secrets/pkg/client/clientset/versioned"
ssscheme "github.com/bitnami-labs/sealed-secrets/pkg/client/clientset/versioned/scheme"
ssv1alpha1client "github.com/bitnami-labs/sealed-secrets/pkg/client/clientset/versioned/typed/sealed-secrets/v1alpha1"
Expand Down Expand Up @@ -66,33 +64,6 @@ type Controller struct {
updateStatus bool // feature flag that enables updating the status subresource.
}

func unseal(ctx context.Context, sclient v1.SecretsGetter, codecs runtimeserializer.CodecFactory, keyRegistry *KeyRegistry, ssecret *ssv1alpha1.SealedSecret) error {
// Important: Be careful not to reveal the namespace/name of
// the *decrypted* Secret (or any other detail) in error/log
// messages.

objName := fmt.Sprintf("%s/%s", ssecret.GetObjectMeta().GetNamespace(), ssecret.GetObjectMeta().GetName())
log.Printf("Updating %s", objName)

secret, err := attemptUnseal(ssecret, keyRegistry)
if err != nil {
// TODO: Add error event
return err
}

_, err = sclient.Secrets(ssecret.GetObjectMeta().GetNamespace()).Create(ctx, secret, metav1.CreateOptions{})
if err != nil && errors.IsAlreadyExists(err) {
_, err = sclient.Secrets(ssecret.GetObjectMeta().GetNamespace()).Update(ctx, secret, metav1.UpdateOptions{})
}
if err != nil {
// TODO: requeue?
return err
}

log.Printf("Updated %s", objName)
return nil
}
juan131 marked this conversation as resolved.
Show resolved Hide resolved

// NewController returns the main sealed-secrets controller loop.
func NewController(clientset kubernetes.Interface, ssclientset ssclientset.Interface, ssinformer ssinformer.SharedInformerFactory, keyRegistry *KeyRegistry) *Controller {
queue := workqueue.NewRateLimitingQueue(workqueue.DefaultControllerRateLimiter())
Expand Down Expand Up @@ -235,11 +206,11 @@ func (c *Controller) unseal(ctx context.Context, key string) (unsealErr error) {
return nil
}

ssecret := obj.(*ssv1alpha1.SealedSecret)
ssecret := obj.(*v1alpha1.SealedSecret)
log.Printf("Updating %s", key)

// any exit of this function at this point will cause an update to the status subresource
// of the SealedSecret custome resource. The return value of the unseal function is available
// of the SealedSecret custom resource. The return value of the unseal function is available
// to the deferred function body in the unsealErr named return value (even if explicit return
// statements are used to return).
defer func() {
Expand Down Expand Up @@ -286,7 +257,7 @@ func (c *Controller) unseal(ctx context.Context, key string) (unsealErr error) {
secret.ObjectMeta.Labels = newSecret.ObjectMeta.Labels

if !apiequality.Semantic.DeepEqual(origSecret, secret) {
secret, err = c.sclient.Secrets(ssecret.GetObjectMeta().GetNamespace()).Update(ctx, secret, metav1.UpdateOptions{})
_, err = c.sclient.Secrets(ssecret.GetObjectMeta().GetNamespace()).Update(ctx, secret, metav1.UpdateOptions{})
if err != nil {
c.recorder.Event(ssecret, corev1.EventTypeWarning, ErrUpdateFailed, err.Error())
unsealErrorsTotal.WithLabelValues("update", ssecret.GetNamespace()).Inc()
Expand All @@ -298,14 +269,14 @@ func (c *Controller) unseal(ctx context.Context, key string) (unsealErr error) {
return nil
}

func (c *Controller) updateSealedSecretStatus(ssecret *ssv1alpha1.SealedSecret, unsealError error) error {
func (c *Controller) updateSealedSecretStatus(ssecret *v1alpha1.SealedSecret, unsealError error) error {
if !c.updateStatus {
klog.V(2).Infof("not updating status because updateStatus feature flag not turned on")
return nil
}

if ssecret.Status == nil {
ssecret.Status = &ssv1alpha1.SealedSecretStatus{}
ssecret.Status = &v1alpha1.SealedSecretStatus{}
}

// No need to update the status if we already have observed it from the
Expand All @@ -321,15 +292,15 @@ func (c *Controller) updateSealedSecretStatus(ssecret *ssv1alpha1.SealedSecret,
return err
}

func updateSealedSecretsStatusConditions(st *ssv1alpha1.SealedSecretStatus, unsealError error) {
cond := func() *ssv1alpha1.SealedSecretCondition {
func updateSealedSecretsStatusConditions(st *v1alpha1.SealedSecretStatus, unsealError error) {
cond := func() *v1alpha1.SealedSecretCondition {
for i := range st.Conditions {
if st.Conditions[i].Type == ssv1alpha1.SealedSecretSynced {
if st.Conditions[i].Type == v1alpha1.SealedSecretSynced {
return &st.Conditions[i]
}
}
st.Conditions = append(st.Conditions, ssv1alpha1.SealedSecretCondition{
Type: ssv1alpha1.SealedSecretSynced,
st.Conditions = append(st.Conditions, v1alpha1.SealedSecretCondition{
Type: v1alpha1.SealedSecretSynced,
})
return &st.Conditions[len(st.Conditions)-1]
}()
Expand All @@ -349,40 +320,9 @@ func updateSealedSecretsStatusConditions(st *ssv1alpha1.SealedSecretStatus, unse
}
}

func (c *Controller) updateSecret(ctx context.Context, newSecret *corev1.Secret) (*corev1.Secret, error) {
existingSecret, err := c.sclient.Secrets(newSecret.GetObjectMeta().GetNamespace()).Get(ctx, newSecret.GetObjectMeta().GetName(), metav1.GetOptions{})
if err != nil {
return nil, fmt.Errorf("failed to read existing secret: %s", err)
}
existingSecret = existingSecret.DeepCopy()
existingSecret.Data = newSecret.Data

c.updateOwnerReferences(existingSecret, newSecret)

return existingSecret, nil
}

func (c *Controller) updateOwnerReferences(existing, new *corev1.Secret) {
ownerRefs := existing.GetOwnerReferences()

for _, newRef := range new.GetOwnerReferences() {
found := false
for _, ref := range ownerRefs {
if newRef.UID == ref.UID {
found = true
break
}
}
if !found {
ownerRefs = append(ownerRefs, newRef)
}
}
existing.SetOwnerReferences(ownerRefs)
}
Comment on lines -352 to -381
Copy link
Collaborator

Choose a reason for hiding this comment

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

@mkmik do you know why is this unused? Tech debt?

Copy link
Collaborator

Choose a reason for hiding this comment

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

it's a leftover from some refactoring, possibly when the k8s client API got updated.

it's dead code, let's remove it


// checks if the annotation equals to "true", and it's case sensitive
// checks if the annotation equals to "true", and it's case-sensitive
func isAnnotatedToBeManaged(secret *corev1.Secret) bool {
return secret.Annotations[ssv1alpha1.SealedSecretManagedAnnotation] == "true"
return secret.Annotations[v1alpha1.SealedSecretManagedAnnotation] == "true"
}

// AttemptUnseal tries to unseal a secret.
Expand All @@ -391,13 +331,13 @@ func (c *Controller) AttemptUnseal(content []byte) (bool, error) {
return false, err
}

object, err := runtime.Decode(scheme.Codecs.UniversalDecoder(ssv1alpha1.SchemeGroupVersion), content)
object, err := runtime.Decode(scheme.Codecs.UniversalDecoder(v1alpha1.SchemeGroupVersion), content)
if err != nil {
return false, err
}

switch s := object.(type) {
case *ssv1alpha1.SealedSecret:
case *v1alpha1.SealedSecret:
if _, err := c.attemptUnseal(s); err != nil {
return false, nil
}
Expand All @@ -411,19 +351,19 @@ func (c *Controller) AttemptUnseal(content []byte) (bool, error) {
// with the latest private key. If the secret is already encrypted with the latest,
// returns the input.
func (c *Controller) Rotate(content []byte) ([]byte, error) {
object, err := runtime.Decode(scheme.Codecs.UniversalDecoder(ssv1alpha1.SchemeGroupVersion), content)
object, err := runtime.Decode(scheme.Codecs.UniversalDecoder(v1alpha1.SchemeGroupVersion), content)
if err != nil {
return nil, err
}

switch s := object.(type) {
case *ssv1alpha1.SealedSecret:
case *v1alpha1.SealedSecret:
secret, err := c.attemptUnseal(s)
if err != nil {
return nil, fmt.Errorf("Error decrypting secret. %v", err)
}
latestPrivKey := c.keyRegistry.latestPrivateKey()
resealedSecret, err := ssv1alpha1.NewSealedSecret(scheme.Codecs, &latestPrivKey.PublicKey, secret)
resealedSecret, err := v1alpha1.NewSealedSecret(scheme.Codecs, &latestPrivKey.PublicKey, secret)
if err != nil {
return nil, fmt.Errorf("Error creating new sealed secret. %v", err)
}
Expand All @@ -437,11 +377,11 @@ func (c *Controller) Rotate(content []byte) ([]byte, error) {
}
}

func (c *Controller) attemptUnseal(ss *ssv1alpha1.SealedSecret) (*corev1.Secret, error) {
func (c *Controller) attemptUnseal(ss *v1alpha1.SealedSecret) (*corev1.Secret, error) {
return attemptUnseal(ss, c.keyRegistry)
}

func attemptUnseal(ss *ssv1alpha1.SealedSecret, keyRegistry *KeyRegistry) (*corev1.Secret, error) {
func attemptUnseal(ss *v1alpha1.SealedSecret, keyRegistry *KeyRegistry) (*corev1.Secret, error) {
privateKeys := map[string]*rsa.PrivateKey{}
for k, v := range keyRegistry.keys {
privateKeys[k] = v.private
Expand Down
4 changes: 0 additions & 4 deletions cmd/controller/funcs.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,6 @@ import (
"time"
)

const (
compromised = "compromised"
)

// ScheduleJobWithTrigger creates a long-running loop that runs a job after an initialDelay
// and then after each period duration.
// It returns a trigger function that runs the job early when called.
Expand Down
11 changes: 2 additions & 9 deletions cmd/controller/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (

"github.com/bitnami-labs/flagenv"
"github.com/bitnami-labs/pflagenv"

ssv1alpha1 "github.com/bitnami-labs/sealed-secrets/pkg/apis/sealed-secrets/v1alpha1"
"github.com/bitnami-labs/sealed-secrets/pkg/buildinfo"
sealedsecrets "github.com/bitnami-labs/sealed-secrets/pkg/client/clientset/versioned"
Expand Down Expand Up @@ -77,16 +78,8 @@ func init() {
}
}

type controller struct {
clientset kubernetes.Interface
}

func initKeyPrefix(keyPrefix string) (string, error) {
prefix, err := validateKeyPrefix(keyPrefix)
if err != nil {
return "", err
}
return prefix, err
return validateKeyPrefix(keyPrefix)
}

func initKeyRegistry(ctx context.Context, client kubernetes.Interface, r io.Reader, namespace, prefix, label string, keysize int) (*KeyRegistry, error) {
Expand Down
4 changes: 2 additions & 2 deletions cmd/controller/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ type secretRotator func([]byte) ([]byte, error)
// all users of a given cluster. It must not leak any secret material.
// The server is started in the background and a handle to it returned so it can be shut down.
func httpserver(cp certProvider, sc secretChecker, sr secretRotator) *http.Server {
httpRateLimiter := rateLimter()
httpRateLimiter := rateLimiter()

mux := http.NewServeMux()

Expand Down Expand Up @@ -116,7 +116,7 @@ func httpserver(cp certProvider, sc secretChecker, sr secretRotator) *http.Serve
return &server
}

func rateLimter() throttled.HTTPRateLimiter {
func rateLimiter() throttled.HTTPRateLimiter {
store, err := memstore.New(65536)
if err != nil {
log.Fatal(err)
Expand Down
12 changes: 5 additions & 7 deletions cmd/kubeseal/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,6 @@ import (
"path/filepath"
"strings"

"github.com/bitnami-labs/sealed-secrets/pkg/buildinfo"
"github.com/bitnami-labs/sealed-secrets/pkg/crypto"
"github.com/bitnami-labs/sealed-secrets/pkg/multidocyaml"
"github.com/google/renameio"
"github.com/mattn/go-isatty"
flag "github.com/spf13/pflag"
Expand All @@ -36,6 +33,10 @@ import (
"k8s.io/client-go/util/keyutil"
"k8s.io/klog/v2"

"github.com/bitnami-labs/sealed-secrets/pkg/buildinfo"
"github.com/bitnami-labs/sealed-secrets/pkg/crypto"
"github.com/bitnami-labs/sealed-secrets/pkg/multidocyaml"

ssv1alpha1 "github.com/bitnami-labs/sealed-secrets/pkg/apis/sealed-secrets/v1alpha1"

// Register Auth providers
Expand Down Expand Up @@ -388,10 +389,7 @@ func reEncryptSealedSecret(ctx context.Context, in io.Reader, out io.Writer, cod
ssecret.SetCreationTimestamp(metav1.Time{})
ssecret.SetDeletionTimestamp(nil)
ssecret.Generation = 0
if err = sealedSecretOutput(out, codecs, ssecret); err != nil {
return err
}
return nil
return sealedSecretOutput(out, codecs, ssecret)
}

func resourceOutput(out io.Writer, codecs runtimeserializer.CodecFactory, gv runtime.GroupVersioner, obj runtime.Object) error {
Expand Down
16 changes: 5 additions & 11 deletions cmd/kubeseal/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,8 @@ import (
"encoding/pem"
goflag "flag"
"fmt"
"io"
"io/ioutil"
"math/big"
mathrand "math/rand"
"net/http"
"net/http/httptest"
"os"
Expand All @@ -22,8 +20,6 @@ import (
"testing"
"time"

ssv1alpha1 "github.com/bitnami-labs/sealed-secrets/pkg/apis/sealed-secrets/v1alpha1"
"github.com/bitnami-labs/sealed-secrets/pkg/crypto"
"github.com/google/go-cmp/cmp"
"github.com/google/go-cmp/cmp/cmpopts"
flag "github.com/spf13/pflag"
Expand All @@ -33,6 +29,9 @@ import (
"k8s.io/client-go/kubernetes/scheme"
certUtil "k8s.io/client-go/util/cert"
"k8s.io/client-go/util/keyutil"

ssv1alpha1 "github.com/bitnami-labs/sealed-secrets/pkg/apis/sealed-secrets/v1alpha1"
"github.com/bitnami-labs/sealed-secrets/pkg/crypto"
)

const testCert = `
Expand Down Expand Up @@ -89,11 +88,6 @@ func TestMain(m *testing.M) {
os.Exit(m.Run())
}

// This is omg-not safe for real crypto use!
func testRand() io.Reader {
return mathrand.New(mathrand.NewSource(42))
}

func tmpfile(t *testing.T, contents []byte) string {
f, err := ioutil.TempFile("", "testdata")
if err != nil {
Expand Down Expand Up @@ -329,7 +323,7 @@ func TestSeal(t *testing.T) {
t.Fatalf("Error encoding: %v", err)
}

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

outbuf := bytes.Buffer{}
if err := seal(&inbuf, &outbuf, scheme.Codecs, key, tc.scope, false, "", ""); err != nil {
Expand Down Expand Up @@ -763,7 +757,7 @@ func TestRaw(t *testing.T) {
}

for i, tc := range testCases {
// encrypt an iteam with data from the testCase and put it
// encrypt an item with data from the testCase and put it
// in a sealed secret with the metadata from the constants above
t.Run(fmt.Sprint(i), func(t *testing.T) {
enc, err := sealTestItem(certFilename, tc.ns, tc.name, secretValue, tc.scope)
Expand Down