diff --git a/operator/pkg/controller/deployment/deployment_controller.go b/operator/pkg/controller/deployment/deployment_controller.go index 39e30e5..59600e2 100644 --- a/operator/pkg/controller/deployment/deployment_controller.go +++ b/operator/pkg/controller/deployment/deployment_controller.go @@ -3,8 +3,10 @@ package deployment import ( "context" "fmt" + "strings" kubeSecrets "github.com/1Password/onepassword-operator/operator/pkg/kubernetessecrets" + "github.com/1Password/onepassword-operator/operator/pkg/onepassword" op "github.com/1Password/onepassword-operator/operator/pkg/onepassword" "github.com/1Password/onepassword-operator/operator/pkg/utils" @@ -114,7 +116,7 @@ func (r *ReconcileDeployment) Reconcile(request reconcile.Request) (reconcile.Re } } // Handles creation or updating secrets for deployment if needed - if err := r.HandleApplyingDeployment(deployment.Namespace, annotations, request); err != nil { + if err := r.HandleApplyingDeployment(deployment, annotations, request); err != nil { return reconcile.Result{}, err } return reconcile.Result{}, nil @@ -187,8 +189,16 @@ func (r *ReconcileDeployment) removeOnePasswordFinalizerFromDeployment(deploymen return r.kubeClient.Update(context.Background(), deployment) } -func (r *ReconcileDeployment) HandleApplyingDeployment(namespace string, annotations map[string]string, request reconcile.Request) error { +func (r *ReconcileDeployment) HandleApplyingDeployment(deployment *appsv1.Deployment, annotations map[string]string, request reconcile.Request) error { reqLog := log.WithValues("Request.Namespace", request.Namespace, "Request.Name", request.Name) + namespace := deployment.Namespace + + // check if deployment is marked to be injected with secrets via the webhook + injectedContainers, injected := annotations[op.ContainerInjectAnnotation] + if injected { + parsedInjectedContainers := strings.Split(injectedContainers, ",") + return onepassword.CreateOnePasswordItemResourceFromDeployment(r.opConnectClient, r.kubeClient, deployment, parsedInjectedContainers) + } secretName := annotations[op.NameAnnotation] secretLabels := map[string]string(nil) diff --git a/operator/pkg/controller/onepassworditem/onepassworditem_controller.go b/operator/pkg/controller/onepassworditem/onepassworditem_controller.go index ad6f3db..9c801bc 100644 --- a/operator/pkg/controller/onepassworditem/onepassworditem_controller.go +++ b/operator/pkg/controller/onepassworditem/onepassworditem_controller.go @@ -148,6 +148,13 @@ func (r *ReconcileOnePasswordItem) HandleOnePasswordItem(resource *onepasswordv1 annotations := resource.Annotations autoRestart := annotations[op.RestartDeploymentsAnnotation] + // do not create kubernetes secret if the OnePasswordItem was generated + // due to secret being injected container via webhook + _, injectedSecret := annotations[op.InjectedAnnotation] + if injectedSecret { + return nil + } + item, err := onepassword.GetOnePasswordItemByPath(r.opConnectClient, resource.Spec.ItemPath) if err != nil { return fmt.Errorf("Failed to retrieve item: %v", err) diff --git a/operator/pkg/kubernetessecrets/kubernetes_secrets_builder.go b/operator/pkg/kubernetessecrets/kubernetes_secrets_builder.go index 734899d..27c7534 100644 --- a/operator/pkg/kubernetessecrets/kubernetes_secrets_builder.go +++ b/operator/pkg/kubernetessecrets/kubernetes_secrets_builder.go @@ -5,7 +5,6 @@ import ( "fmt" "regexp" - "strings" "reflect" @@ -76,7 +75,7 @@ func CreateKubernetesSecretFromItem(kubeClient kubernetesClient.Client, secretNa func BuildKubernetesSecretFromOnePasswordItem(name, namespace string, annotations map[string]string, labels map[string]string, item onepassword.Item) *corev1.Secret { return &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ - Name: formatSecretName(name), + Name: utils.FormatSecretName(name), Namespace: namespace, Annotations: annotations, Labels: labels, @@ -96,17 +95,6 @@ func BuildKubernetesSecretData(fields []*onepassword.ItemField) map[string][]byt return secretData } -// formatSecretName rewrites a value to be a valid Secret name. -// -// The Secret meta.name and data keys must be valid DNS subdomain names -// (https://kubernetes.io/docs/concepts/configuration/secret/#overview-of-secrets) -func formatSecretName(value string) string { - if errs := kubeValidate.IsDNS1123Subdomain(value); len(errs) == 0 { - return value - } - return createValidSecretName(value) -} - // formatSecretDataName rewrites a value to be a valid Secret data key. // // The Secret data keys must consist of alphanumeric numbers, `-`, `_` or `.` @@ -118,20 +106,6 @@ func formatSecretDataName(value string) string { return createValidSecretDataName(value) } -var invalidDNS1123Chars = regexp.MustCompile("[^a-z0-9-.]+") - -func createValidSecretName(value string) string { - result := strings.ToLower(value) - result = invalidDNS1123Chars.ReplaceAllString(result, "-") - - if len(result) > kubeValidate.DNS1123SubdomainMaxLength { - result = result[0:kubeValidate.DNS1123SubdomainMaxLength] - } - - // first and last character MUST be alphanumeric - return strings.Trim(result, "-.") -} - var invalidDataChars = regexp.MustCompile("[^a-zA-Z0-9-._]+") var invalidStartEndChars = regexp.MustCompile("(^[^a-zA-Z0-9-._]+|[^a-zA-Z0-9-._]+$)") diff --git a/operator/pkg/onepassword/annotations.go b/operator/pkg/onepassword/annotations.go index 652f671..8273e66 100644 --- a/operator/pkg/onepassword/annotations.go +++ b/operator/pkg/onepassword/annotations.go @@ -14,6 +14,8 @@ const ( VersionAnnotation = OnepasswordPrefix + "/item-version" RestartAnnotation = OnepasswordPrefix + "/last-restarted" RestartDeploymentsAnnotation = OnepasswordPrefix + "/auto-restart" + ContainerInjectAnnotation = OnepasswordPrefix + "/inject" + InjectedAnnotation = OnepasswordPrefix + "/injected" ) func GetAnnotationsForDeployment(deployment *appsv1.Deployment, regex *regexp.Regexp) (map[string]string, bool) { diff --git a/operator/pkg/onepassword/containers.go b/operator/pkg/onepassword/containers.go index 1f51bd9..cc4fb98 100644 --- a/operator/pkg/onepassword/containers.go +++ b/operator/pkg/onepassword/containers.go @@ -1,6 +1,10 @@ package onepassword -import corev1 "k8s.io/api/core/v1" +import ( + onepasswordv1 "github.com/1Password/onepassword-operator/operator/pkg/apis/onepassword/v1" + "github.com/1Password/onepassword-operator/operator/pkg/utils" + corev1 "k8s.io/api/core/v1" +) func AreContainersUsingSecrets(containers []corev1.Container, secrets map[string]*corev1.Secret) bool { for i := 0; i < len(containers); i++ { @@ -31,3 +35,29 @@ func AppendUpdatedContainerSecrets(containers []corev1.Container, secrets map[st } return updatedDeploymentSecrets } + +func AreContainersUsingInjectedSecrets(containers []corev1.Container, injectedContainers []string, items map[string]*onepasswordv1.OnePasswordItem) bool { + for _, container := range containers { + envVariables := container.Env + + // check if container was set to be injected with secrets + for _, injectedContainer := range injectedContainers { + if injectedContainer != container.Name { + continue + } + } + + // check if any environment variables are using an updated injected secret + for _, envVariable := range envVariables { + referenceVault, referenceItem, err := ParseReference(envVariable.Value) + if err != nil { + continue + } + _, itemFound := items[utils.BuildInjectedOnePasswordItemName(referenceVault, referenceItem)] + if itemFound { + return true + } + } + } + return false +} diff --git a/operator/pkg/onepassword/deployments.go b/operator/pkg/onepassword/deployments.go index 6c97efb..ce523be 100644 --- a/operator/pkg/onepassword/deployments.go +++ b/operator/pkg/onepassword/deployments.go @@ -1,6 +1,9 @@ package onepassword import ( + "strings" + + onepasswordv1 "github.com/1Password/onepassword-operator/operator/pkg/apis/onepassword/v1" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" ) @@ -24,3 +27,14 @@ func GetUpdatedSecretsForDeployment(deployment *appsv1.Deployment, secrets map[s return updatedSecretsForDeployment } + +func IsDeploymentUsingInjectedSecrets(deployment *appsv1.Deployment, items map[string]*onepasswordv1.OnePasswordItem) bool { + containers := deployment.Spec.Template.Spec.Containers + containers = append(containers, deployment.Spec.Template.Spec.InitContainers...) + injectedContainers, enabled := deployment.Spec.Template.Annotations[ContainerInjectAnnotation] + if !enabled { + return false + } + parsedInjectedContainers := strings.Split(injectedContainers, ",") + return AreContainersUsingInjectedSecrets(containers, parsedInjectedContainers, items) +} diff --git a/operator/pkg/onepassword/items.go b/operator/pkg/onepassword/items.go index 11c4914..e088205 100644 --- a/operator/pkg/onepassword/items.go +++ b/operator/pkg/onepassword/items.go @@ -11,6 +11,16 @@ import ( var logger = logf.Log.WithName("retrieve_item") +const secretReferencePrefix = "op://" + +type InvalidOPFormatError struct { + Reference string +} + +func (e *InvalidOPFormatError) Error() string { + return fmt.Sprintf("Invalid secret reference : %s. Secret references should start with op://", e.Reference) +} + func GetOnePasswordItemByPath(opConnectClient connect.Client, path string) (*onepassword.Item, error) { vaultValue, itemValue, err := ParseVaultAndItemFromPath(path) if err != nil { @@ -33,6 +43,30 @@ func GetOnePasswordItemByPath(opConnectClient connect.Client, path string) (*one return item, nil } +func ParseReference(reference string) (string, string, error) { + if !strings.HasPrefix(reference, secretReferencePrefix) { + return "", "", &InvalidOPFormatError{Reference: reference} + } + path := strings.TrimPrefix(reference, secretReferencePrefix) + + splitPath := strings.Split(path, "/") + if len(splitPath) != 3 { + return "", "", fmt.Errorf("Invalid secret reference : %s. Secret references should match op:////", reference) + } + + vault := splitPath[0] + if vault == "" { + return "", "", fmt.Errorf("Invalid secret reference : %s. Vault can't be empty.", reference) + } + + item := splitPath[1] + if item == "" { + return "", "", fmt.Errorf("Invalid secret reference : %s. Item can't be empty.", reference) + } + + return vault, item, nil +} + func ParseVaultAndItemFromPath(path string) (string, string, error) { splitPath := strings.Split(path, "/") if len(splitPath) == 4 && splitPath[0] == "vaults" && splitPath[2] == "items" { diff --git a/operator/pkg/onepassword/onepassword_item.go b/operator/pkg/onepassword/onepassword_item.go new file mode 100644 index 0000000..14db69a --- /dev/null +++ b/operator/pkg/onepassword/onepassword_item.go @@ -0,0 +1,95 @@ +package onepassword + +import ( + "context" + "errors" + "fmt" + + "github.com/1Password/connect-sdk-go/connect" + onepasswordv1 "github.com/1Password/onepassword-operator/operator/pkg/apis/onepassword/v1" + "github.com/1Password/onepassword-operator/operator/pkg/utils" + k8sErrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + + appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" + kubernetesClient "sigs.k8s.io/controller-runtime/pkg/client" +) + +func CreateOnePasswordItemResourceFromDeployment(opClient connect.Client, kubeClient kubernetesClient.Client, deployment *appsv1.Deployment, injectedContainers []string) error { + containers := deployment.Spec.Template.Spec.Containers + containers = append(containers, deployment.Spec.Template.Spec.InitContainers...) + for _, container := range containers { + // check if container is listed is one of the containers + // set to have injected secrets + for _, injectedContainer := range injectedContainers { + if injectedContainer != container.Name { + continue + } + // create a one password item custom resource to track updates for injected secrets + err := CreateOnePasswordCRSecretsFromContainer(opClient, kubeClient, container, deployment.Namespace) + if err != nil { + return err + } + } + } + return nil +} + +func CreateOnePasswordCRSecretsFromContainer(opClient connect.Client, kubeClient kubernetesClient.Client, container corev1.Container, namespace string) error { + for _, env := range container.Env { + // if value is not of format op://// then ignore + vault, item, err := ParseReference(env.Value) + if err != nil { + var ev *InvalidOPFormatError + if !errors.As(err, &ev) { + return err + } + continue + } + // create a one password item custom resource to track updates for injected secrets + err = CreateOnePasswordCRSecretFromReference(opClient, kubeClient, vault, item, namespace) + if err != nil { + return err + } + } + return nil +} + +func CreateOnePasswordCRSecretFromReference(opClient connect.Client, kubeClient kubernetesClient.Client, vault, item, namespace string) error { + + retrievedItem, err := GetOnePasswordItemByPath(opClient, fmt.Sprintf("vaults/%s/items/%s", vault, item)) + if err != nil { + return fmt.Errorf("Failed to retrieve item: %v", err) + } + + name := utils.BuildInjectedOnePasswordItemName(vault, item) + onepassworditem := BuildOnePasswordItemCRFromPath(vault, item, name, namespace, fmt.Sprint(retrievedItem.Version)) + + currentOnepassworditem := &onepasswordv1.OnePasswordItem{} + err = kubeClient.Get(context.Background(), types.NamespacedName{Name: onepassworditem.Name, Namespace: onepassworditem.Namespace}, currentOnepassworditem) + if k8sErrors.IsNotFound(err) { + log.Info(fmt.Sprintf("Creating OnePasswordItem CR %v at namespace '%v'", onepassworditem.Name, onepassworditem.Namespace)) + return kubeClient.Create(context.Background(), onepassworditem) + } else if err != nil { + return err + } + return nil +} + +func BuildOnePasswordItemCRFromPath(vault, item, name, namespace, version string) *onepasswordv1.OnePasswordItem { + return &onepasswordv1.OnePasswordItem{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + Annotations: map[string]string{ + InjectedAnnotation: "true", + VersionAnnotation: version, + }, + }, + Spec: onepasswordv1.OnePasswordItemSpec{ + ItemPath: fmt.Sprintf("vaults/%s/items/%s", vault, item), + }, + } +} diff --git a/operator/pkg/onepassword/onepassword_item_test.go b/operator/pkg/onepassword/onepassword_item_test.go new file mode 100644 index 0000000..0c9a24e --- /dev/null +++ b/operator/pkg/onepassword/onepassword_item_test.go @@ -0,0 +1,234 @@ +package onepassword + +import ( + "context" + "fmt" + "strings" + "testing" + + "github.com/1Password/onepassword-operator/operator/pkg/mocks" + + "github.com/1Password/connect-sdk-go/onepassword" + onepasswordv1 "github.com/1Password/onepassword-operator/operator/pkg/apis/onepassword/v1" + "github.com/stretchr/testify/assert" + appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" + errors2 "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + "k8s.io/kubectl/pkg/scheme" + "sigs.k8s.io/controller-runtime/pkg/client/fake" +) + +type onepassworditemInjections struct { + testName string + existingDeployment *appsv1.Deployment + existingNamespace *corev1.Namespace + expectedError error + expectedEvents []string + opItem map[string]string + expectedOPItem *onepasswordv1.OnePasswordItem +} + +var onepassworditemTests = []onepassworditemInjections{ + { + testName: "Try to Create OnePasswordItem with container with valid op reference", + existingNamespace: defaultNamespace, + existingDeployment: &appsv1.Deployment{ + TypeMeta: metav1.TypeMeta{ + Kind: deploymentKind, + APIVersion: deploymentAPIVersion, + }, + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + }, + Spec: appsv1.DeploymentSpec{ + Template: corev1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + Annotations: map[string]string{ + ContainerInjectAnnotation: "test-app", + }, + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "test-app", + Env: []corev1.EnvVar{ + { + Name: name, + Value: fmt.Sprintf("op://%s/%s/test", vaultId, itemId), + }, + }, + }, + }, + }, + }, + }, + }, + expectedError: nil, + opItem: map[string]string{ + userKey: username, + passKey: password, + }, + expectedOPItem: &onepasswordv1.OnePasswordItem{ + TypeMeta: metav1.TypeMeta{ + Kind: "OnePasswordItem", + APIVersion: "onepassword.com/v1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: injectedOnePasswordItemName, + Namespace: namespace, + Annotations: map[string]string{ + InjectedAnnotation: "true", + VersionAnnotation: "old", + }, + }, + Spec: onepasswordv1.OnePasswordItemSpec{ + ItemPath: itemPath, + }, + }, + }, + { + testName: "Container with no op:// reference does not create OnePasswordItem", + existingNamespace: defaultNamespace, + existingDeployment: &appsv1.Deployment{ + TypeMeta: metav1.TypeMeta{ + Kind: deploymentKind, + APIVersion: deploymentAPIVersion, + }, + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + }, + Spec: appsv1.DeploymentSpec{ + Template: corev1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + Annotations: map[string]string{ + ContainerInjectAnnotation: "test-app", + }, + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "test-app", + Env: []corev1.EnvVar{ + { + Name: name, + Value: fmt.Sprintf("some value"), + }, + }, + }, + }, + }, + }, + }, + }, + expectedError: nil, + opItem: map[string]string{ + userKey: username, + passKey: password, + }, + expectedOPItem: nil, + }, + { + testName: "Container with op:// reference missing vault and item does not create OnePasswordItem and returns error", + existingNamespace: defaultNamespace, + existingDeployment: &appsv1.Deployment{ + TypeMeta: metav1.TypeMeta{ + Kind: deploymentKind, + APIVersion: deploymentAPIVersion, + }, + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + }, + Spec: appsv1.DeploymentSpec{ + Template: corev1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + Annotations: map[string]string{ + ContainerInjectAnnotation: "test-app", + }, + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "test-app", + Env: []corev1.EnvVar{ + { + Name: name, + Value: fmt.Sprintf("op://"), + }, + }, + }, + }, + }, + }, + }, + }, + expectedError: fmt.Errorf("Invalid secret reference : %s. Secret references should match op:////", "op://"), + opItem: map[string]string{ + userKey: username, + passKey: password, + }, + expectedOPItem: nil, + }, +} + +func TestOnePasswordItemSecretInjected(t *testing.T) { + for _, testData := range onepassworditemTests { + t.Run(testData.testName, func(t *testing.T) { + + // Register operator types with the runtime scheme. + s := scheme.Scheme + s.AddKnownTypes(appsv1.SchemeGroupVersion, &onepasswordv1.OnePasswordItem{}, &onepasswordv1.OnePasswordItemList{}, &appsv1.Deployment{}) + + // Objects to track in the fake client. + objs := []runtime.Object{ + testData.existingDeployment, + testData.existingNamespace, + } + + // Create a fake client to mock API calls. + cl := fake.NewFakeClientWithScheme(s, objs...) + + opConnectClient := &mocks.TestClient{} + mocks.GetGetItemFunc = func(uuid string, vaultUUID string) (*onepassword.Item, error) { + + item := onepassword.Item{} + item.Fields = generateFields(testData.opItem["username"], testData.opItem["password"]) + item.Version = itemVersion + item.Vault.ID = vaultUUID + item.ID = uuid + return &item, nil + } + + injectedContainers := testData.existingDeployment.Spec.Template.ObjectMeta.Annotations[ContainerInjectAnnotation] + parsedInjectedContainers := strings.Split(injectedContainers, ",") + err := CreateOnePasswordItemResourceFromDeployment(opConnectClient, cl, testData.existingDeployment, parsedInjectedContainers) + + assert.Equal(t, testData.expectedError, err) + + // Check if Secret has been created and has the correct data + opItemCR := &onepasswordv1.OnePasswordItem{} + err = cl.Get(context.TODO(), types.NamespacedName{Name: injectedOnePasswordItemName, Namespace: namespace}, opItemCR) + + if testData.expectedOPItem == nil { + assert.Error(t, err) + assert.True(t, errors2.IsNotFound(err)) + } else { + assert.Equal(t, testData.expectedOPItem.Spec.ItemPath, opItemCR.Spec.ItemPath) + assert.Equal(t, testData.expectedOPItem.Name, opItemCR.Name) + assert.Equal(t, testData.expectedOPItem.Annotations[InjectedAnnotation], opItemCR.Annotations[InjectedAnnotation]) + } + + }) + } +} diff --git a/operator/pkg/onepassword/secret_update_handler.go b/operator/pkg/onepassword/secret_update_handler.go index 899c188..d76245c 100644 --- a/operator/pkg/onepassword/secret_update_handler.go +++ b/operator/pkg/onepassword/secret_update_handler.go @@ -5,6 +5,7 @@ import ( "fmt" "time" + onepasswordv1 "github.com/1Password/onepassword-operator/operator/pkg/apis/onepassword/v1" kubeSecrets "github.com/1Password/onepassword-operator/operator/pkg/kubernetessecrets" "github.com/1Password/onepassword-operator/operator/pkg/utils" @@ -41,12 +42,17 @@ func (h *SecretUpdateHandler) UpdateKubernetesSecretsTask() error { return err } - return h.restartDeploymentsWithUpdatedSecrets(updatedKubernetesSecrets) + updatedInjectedSecrets, err := h.updateInjectedSecrets() + if err != nil { + return err + } + + return h.restartDeploymentsWithUpdatedSecrets(updatedKubernetesSecrets, updatedInjectedSecrets) } -func (h *SecretUpdateHandler) restartDeploymentsWithUpdatedSecrets(updatedSecretsByNamespace map[string]map[string]*corev1.Secret) error { - // No secrets to update. Exit - if len(updatedSecretsByNamespace) == 0 || updatedSecretsByNamespace == nil { +func (h *SecretUpdateHandler) restartDeploymentsWithUpdatedSecrets(updatedSecretsByNamespace map[string]map[string]*corev1.Secret, updatedInjectedSecretsByNamespace map[string]map[string]*onepasswordv1.OnePasswordItem) error { + + if len(updatedSecretsByNamespace) == 0 && len(updatedInjectedSecretsByNamespace) == 0 { return nil } @@ -63,6 +69,7 @@ func (h *SecretUpdateHandler) restartDeploymentsWithUpdatedSecrets(updatedSecret setForAutoRestartByNamespaceMap, err := h.getIsSetForAutoRestartByNamespaceMap() if err != nil { + log.Error(err, "Error determining which namespaces allow restarts") return err } @@ -70,17 +77,24 @@ func (h *SecretUpdateHandler) restartDeploymentsWithUpdatedSecrets(updatedSecret deployment := &deployments.Items[i] updatedSecrets := updatedSecretsByNamespace[deployment.Namespace] + // check if deployment is using one of the updated secrets updatedDeploymentSecrets := GetUpdatedSecretsForDeployment(deployment, updatedSecrets) - if len(updatedDeploymentSecrets) == 0 { - continue - } - for _, secret := range updatedDeploymentSecrets { - if isSecretSetForAutoRestart(secret, deployment, setForAutoRestartByNamespaceMap) { - h.restartDeployment(deployment) - continue + if len(updatedDeploymentSecrets) != 0 { + for _, secret := range updatedDeploymentSecrets { + if isSecretSetForAutoRestart(secret, deployment, setForAutoRestartByNamespaceMap) { + h.restartDeployment(deployment) + continue + } } } + // check if the deployment is using one of the updated injected secrets + updatedInjection := IsDeploymentUsingInjectedSecrets(deployment, updatedInjectedSecretsByNamespace[deployment.Namespace]) + if updatedInjection && isDeploymentSetForAutoRestart(deployment, setForAutoRestartByNamespaceMap) { + h.restartDeployment(deployment) + continue + } + log.Info(fmt.Sprintf("Deployment %q at namespace %q is up to date", deployment.GetName(), deployment.Namespace)) } @@ -89,9 +103,10 @@ func (h *SecretUpdateHandler) restartDeploymentsWithUpdatedSecrets(updatedSecret func (h *SecretUpdateHandler) restartDeployment(deployment *appsv1.Deployment) { log.Info(fmt.Sprintf("Deployment %q at namespace %q references an updated secret. Restarting", deployment.GetName(), deployment.Namespace)) - deployment.Spec.Template.Annotations = map[string]string{ - RestartAnnotation: time.Now().String(), + if deployment.Spec.Template.Annotations == nil { + deployment.Spec.Template.Annotations = map[string]string{} } + deployment.Spec.Template.Annotations[RestartAnnotation] = time.Now().String() err := h.client.Update(context.Background(), deployment) if err != nil { log.Error(err, "Problem restarting deployment") @@ -142,6 +157,52 @@ func (h *SecretUpdateHandler) updateKubernetesSecrets() (map[string]map[string]* return updatedSecrets, nil } +func (h *SecretUpdateHandler) updateInjectedSecrets() (map[string]map[string]*onepasswordv1.OnePasswordItem, error) { + // fetch all onepassworditems + onepasswordItems := &onepasswordv1.OnePasswordItemList{} + err := h.client.List(context.Background(), onepasswordItems) + if err != nil { + log.Error(err, "Failed to list OnePasswordItems") + return nil, err + } + + updatedItems := map[string]map[string]*onepasswordv1.OnePasswordItem{} + for _, item := range onepasswordItems.Items { + + // if onepassworditem was not generated by injecting a secret into a deployment then ignore + _, injected := item.Annotations[InjectedAnnotation] + if !injected { + continue + } + + itemPath := item.Spec.ItemPath + currentVersion := item.Annotations[VersionAnnotation] + if len(itemPath) == 0 || len(currentVersion) == 0 { + continue + } + + storedItem, err := GetOnePasswordItemByPath(h.opConnectClient, itemPath) + if err != nil { + return nil, fmt.Errorf("Failed to retrieve item: %v", err) + } + + itemVersion := fmt.Sprint(storedItem.Version) + if currentVersion != itemVersion { + item.Annotations[VersionAnnotation] = itemVersion + h.client.Update(context.Background(), &item) + if isItemLockedForForcedRestarts(storedItem) { + log.Info(fmt.Sprintf("Secret '%v' has been updated in 1Password but is set to be ignored. Updates to an ignored secret will not trigger an update to a OnePasswordItem secret or a rolling restart.", item.Name)) + continue + } + if updatedItems[item.Namespace] == nil { + updatedItems[item.Namespace] = make(map[string]*onepasswordv1.OnePasswordItem) + } + updatedItems[item.Namespace][item.Name] = &item + } + } + return updatedItems, nil +} + func isItemLockedForForcedRestarts(item *onepassword.Item) bool { tags := item.Tags for i := 0; i < len(tags); i++ { @@ -178,7 +239,7 @@ func (h *SecretUpdateHandler) getIsSetForAutoRestartByNamespaceMap() (map[string func isSecretSetForAutoRestart(secret *corev1.Secret, deployment *appsv1.Deployment, setForAutoRestartByNamespace map[string]bool) bool { restartDeployment := secret.Annotations[RestartDeploymentsAnnotation] - //If annotation for auto restarts for deployment is not set. Check for the annotation on its namepsace + //If annotation for auto restarts for deployment is not set. Check for the annotation on its deployment if restartDeployment == "" { return isDeploymentSetForAutoRestart(deployment, setForAutoRestartByNamespace) } diff --git a/operator/pkg/onepassword/secret_update_handler_test.go b/operator/pkg/onepassword/secret_update_handler_test.go index 4155e4e..cfb1077 100644 --- a/operator/pkg/onepassword/secret_update_handler_test.go +++ b/operator/pkg/onepassword/secret_update_handler_test.go @@ -8,6 +8,7 @@ import ( "github.com/1Password/onepassword-operator/operator/pkg/mocks" "github.com/1Password/connect-sdk-go/onepassword" + onepasswordv1 "github.com/1Password/onepassword-operator/operator/pkg/apis/onepassword/v1" "github.com/stretchr/testify/assert" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" @@ -20,23 +21,25 @@ import ( ) const ( - deploymentKind = "Deployment" - deploymentAPIVersion = "v1" - name = "test-deployment" - namespace = "default" - vaultId = "hfnjvi6aymbsnfc2xeeoheizda" - itemId = "nwrhuano7bcwddcviubpp4mhfq" - username = "test-user" - password = "QmHumKc$mUeEem7caHtbaBaJ" - userKey = "username" - passKey = "password" - itemVersion = 123 + deploymentKind = "Deployment" + deploymentAPIVersion = "v1" + name = "test-deployment" + namespace = "default" + vaultId = "hfnjvi6aymbsnfc2xeeoheizda" + itemId = "nwrhuano7bcwddcviubpp4mhfq" + username = "test-user" + password = "QmHumKc$mUeEem7caHtbaBaJ" + userKey = "username" + passKey = "password" + itemVersion = 123 + injectedOnePasswordItemName = "injectedsecret-" + vaultId + "-" + itemId ) type testUpdateSecretTask struct { testName string existingDeployment *appsv1.Deployment existingNamespace *corev1.Namespace + existingOnePasswordItem *onepasswordv1.OnePasswordItem existingSecret *corev1.Secret expectedError error expectedResultSecret *corev1.Secret @@ -755,6 +758,123 @@ var tests = []testUpdateSecretTask{ expectedRestart: true, globalAutoRestartEnabled: false, }, + { + testName: "OP item has new version. Secret needs update. Deployment is restarted based on injected secrets in containers", + existingNamespace: defaultNamespace, + existingDeployment: &appsv1.Deployment{ + TypeMeta: metav1.TypeMeta{ + Kind: deploymentKind, + APIVersion: deploymentAPIVersion, + }, + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + }, + Spec: appsv1.DeploymentSpec{ + Template: corev1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + Annotations: map[string]string{ + ContainerInjectAnnotation: "test-app", + }, + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "test-app", + Env: []corev1.EnvVar{ + { + Name: name, + Value: fmt.Sprintf("op://%s/%s/test", vaultId, itemId), + }, + }, + }, + }, + }, + }, + }, + }, + existingOnePasswordItem: &onepasswordv1.OnePasswordItem{ + TypeMeta: metav1.TypeMeta{ + Kind: "OnePasswordItem", + APIVersion: "onepassword.com/v1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: injectedOnePasswordItemName, + Namespace: namespace, + Annotations: map[string]string{ + InjectedAnnotation: "true", + VersionAnnotation: "old", + }, + }, + Spec: onepasswordv1.OnePasswordItemSpec{ + ItemPath: itemPath, + }, + }, + expectedError: nil, + opItem: map[string]string{ + userKey: username, + passKey: password, + }, + expectedRestart: true, + globalAutoRestartEnabled: true, + }, + { + testName: "OP item has new version. Secret needs update. Deployment does not have a inject annotation", + existingNamespace: defaultNamespace, + existingDeployment: &appsv1.Deployment{ + TypeMeta: metav1.TypeMeta{ + Kind: deploymentKind, + APIVersion: deploymentAPIVersion, + }, + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + }, + Spec: appsv1.DeploymentSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "test-app", + Env: []corev1.EnvVar{ + { + Name: name, + Value: fmt.Sprintf("op://%s/%s/test", vaultId, itemId), + }, + }, + }, + }, + }, + }, + }, + }, + existingOnePasswordItem: &onepasswordv1.OnePasswordItem{ + TypeMeta: metav1.TypeMeta{ + Kind: "OnePasswordItem", + APIVersion: "onepassword.com/v1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: fmt.Sprintf("%s-%s", vaultId, itemId), + Namespace: namespace, + Annotations: map[string]string{ + InjectedAnnotation: "true", + VersionAnnotation: "old", + }, + }, + Spec: onepasswordv1.OnePasswordItemSpec{ + ItemPath: itemPath, + }, + }, + expectedError: nil, + opItem: map[string]string{ + userKey: username, + passKey: password, + }, + expectedRestart: false, + globalAutoRestartEnabled: true, + }, } func TestUpdateSecretHandler(t *testing.T) { @@ -763,7 +883,7 @@ func TestUpdateSecretHandler(t *testing.T) { // Register operator types with the runtime scheme. s := scheme.Scheme - s.AddKnownTypes(appsv1.SchemeGroupVersion, testData.existingDeployment) + s.AddKnownTypes(appsv1.SchemeGroupVersion, &onepasswordv1.OnePasswordItem{}, &onepasswordv1.OnePasswordItemList{}, &appsv1.Deployment{}) // Objects to track in the fake client. objs := []runtime.Object{ @@ -775,6 +895,10 @@ func TestUpdateSecretHandler(t *testing.T) { objs = append(objs, testData.existingSecret) } + if testData.existingOnePasswordItem != nil { + objs = append(objs, testData.existingOnePasswordItem) + } + // Create a fake client to mock API calls. cl := fake.NewFakeClientWithScheme(s, objs...) @@ -825,9 +949,9 @@ func TestUpdateSecretHandler(t *testing.T) { _, ok := deployment.Spec.Template.Annotations[RestartAnnotation] if ok { - assert.True(t, testData.expectedRestart, "Expected deployment to restart but it did not") + assert.True(t, testData.expectedRestart, "Deployment was restarted but should not have been.") } else { - assert.False(t, testData.expectedRestart, "Deployment was restarted but should not have been.") + assert.False(t, testData.expectedRestart, "Expected deployment to restart but it did not") } }) } diff --git a/operator/pkg/utils/string.go b/operator/pkg/utils/string.go index ffe2871..8b372c3 100644 --- a/operator/pkg/utils/string.go +++ b/operator/pkg/utils/string.go @@ -1,10 +1,16 @@ package utils import ( + "fmt" + "regexp" "strconv" "strings" + + kubeValidate "k8s.io/apimachinery/pkg/util/validation" ) +var invalidDNS1123Chars = regexp.MustCompile("[^a-z0-9-.]+") + func ContainsString(slice []string, s string) bool { for _, item := range slice { if item == s { @@ -31,3 +37,30 @@ func StringToBool(str string) (bool, error) { } return restartDeploymentBool, nil } + +// formatSecretName rewrites a value to be a valid Secret name. +// +// The Secret meta.name and data keys must be valid DNS subdomain names +// (https://kubernetes.io/docs/concepts/configuration/secret/#overview-of-secrets) +func FormatSecretName(value string) string { + if errs := kubeValidate.IsDNS1123Subdomain(value); len(errs) == 0 { + return value + } + return CreateValidSecretName(value) +} + +func CreateValidSecretName(value string) string { + result := strings.ToLower(value) + result = invalidDNS1123Chars.ReplaceAllString(result, "-") + + if len(result) > kubeValidate.DNS1123SubdomainMaxLength { + result = result[0:kubeValidate.DNS1123SubdomainMaxLength] + } + + // first and last character MUST be alphanumeric + return strings.Trim(result, "-.") +} + +func BuildInjectedOnePasswordItemName(vaultId, injectedId string) string { + return FormatSecretName(fmt.Sprintf("injectedsecret-%s-%s", vaultId, injectedId)) +}