Skip to content

Commit

Permalink
feature: add BYOID support for OCI artifacts and Helm charts
Browse files Browse the repository at this point in the history
This commit integrates BYOID support into Config Sync by supporting a
new auth type for OCI and Helm: `k8sserviceaccount`. It requires GKE or
Fleet Workload Identity to be enabled. The KSA along with WI will be
exchanged to a federated token which can be used to authenticate to
Ubermint-enabled GCP services directly.

Currently, Artifact Registry supports Ubermint. Cloud Source
Repositories will be replaced by Secure Source Manager, which will
support Ubermint in Q1 2024.
  • Loading branch information
nan-yu committed Feb 2, 2024
1 parent 2509692 commit 2b2eed4
Show file tree
Hide file tree
Showing 27 changed files with 652 additions and 201 deletions.
4 changes: 2 additions & 2 deletions cmd/helm-sync/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,8 @@ var (
flIncludeCRDs = flag.String("include-crds", os.Getenv(reconcilermanager.HelmIncludeCRDs),
"include CRDs in the helm rendering output")
flAuth = flag.String("auth", util.EnvString(reconcilermanager.HelmAuthType, string(configsync.AuthNone)),
fmt.Sprintf("the authentication type for access to the Helm repository. Must be one of %s, %s, %s or %s. Defaults to %s",
configsync.AuthGCPServiceAccount, configsync.AuthToken, configsync.AuthGCENode, configsync.AuthNone, configsync.AuthNone))
fmt.Sprintf("the authentication type for access to the Helm repository. Must be one of %s, %s, %s, %s or %s. Defaults to %s",
configsync.AuthGCPServiceAccount, configsync.AuthK8sServiceAccount, configsync.AuthToken, configsync.AuthGCENode, configsync.AuthNone, configsync.AuthNone))
flReleaseName = flag.String("release-name", os.Getenv(reconcilermanager.HelmReleaseName),
"the name of helm release")
flNamespace = flag.String("namespace", os.Getenv(reconcilermanager.HelmReleaseNamespace),
Expand Down
4 changes: 2 additions & 2 deletions cmd/oci-sync/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,8 @@ import (
var flImage = flag.String("image", util.EnvString(reconcilermanager.OciSyncImage, ""),
"the OCI image repository for the package")
var flAuth = flag.String("auth", util.EnvString(reconcilermanager.OciSyncAuth, string(configsync.AuthNone)),
fmt.Sprintf("the authentication type for access to the OCI package. Must be one of %s, %s, or %s. Defaults to %s",
configsync.AuthGCPServiceAccount, configsync.AuthGCENode, configsync.AuthNone, configsync.AuthNone))
fmt.Sprintf("the authentication type for access to the OCI package. Must be one of %s, %s, %s, or %s. Defaults to %s",
configsync.AuthGCPServiceAccount, configsync.AuthK8sServiceAccount, configsync.AuthGCENode, configsync.AuthNone, configsync.AuthNone))
var flRoot = flag.String("root", util.EnvString("OCI_SYNC_ROOT", util.EnvString("HOME", "")+"/oci"),
"the root directory for oci-sync operations, under which --dest will be created")
var flDest = flag.String("dest", util.EnvString("OCI_SYNC_DEST", ""),
Expand Down
2 changes: 1 addition & 1 deletion e2e/nomostest/nt.go
Original file line number Diff line number Diff line change
Expand Up @@ -971,7 +971,7 @@ func gitCommitFromSpec(nt *NT, gitSpec *v1beta1.Git) (string, error) {
pattern = "HEAD"
}
var args []string
if strings.Contains(gitSpec.Repo, "https://source.developers.google.com") {
if strings.Contains(gitSpec.Repo, testing.CSRHost) {
cloneDir, err := cloneCloudSourceRepo(nt, gitSpec.Repo)
if err != nil {
return "", err
Expand Down
15 changes: 12 additions & 3 deletions e2e/nomostest/testing/env.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,19 @@
package testing

const (
// ARHost is the host address of the Artifact Registry
ARHost = "us-docker.pkg.dev"

// GCRHost is the host address of the Container Registry
GCRHost = "gcr.io"

// CSRHost is the host address of the Cloud Source Repositories
CSRHost = "https://source.developers.google.com"

// TestInfraContainerRegistry is the Config Sync test-infra registry hosted on GCR
TestInfraContainerRegistry = "gcr.io/kpt-config-sync-ci-artifacts"
TestInfraContainerRegistry = GCRHost + "/kpt-config-sync-ci-artifacts"
// TestInfraArtifactRegistry is the Config Sync test-infra registry hosted on GAR
TestInfraArtifactRegistry = "us-docker.pkg.dev/kpt-config-sync-ci-artifacts/test-infra"
TestInfraArtifactRegistry = ARHost + "/kpt-config-sync-ci-artifacts/test-infra"
// ConfigSyncTestPublicRegistry is the Config Sync config-sync-test-public registry hosted on GAR
ConfigSyncTestPublicRegistry = "us-docker.pkg.dev/kpt-config-sync-ci-artifacts/config-sync-test-public"
ConfigSyncTestPublicRegistry = ARHost + "/kpt-config-sync-ci-artifacts/config-sync-test-public"
)
5 changes: 3 additions & 2 deletions e2e/nomostest/testutils/fleet_membership.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
corev1 "k8s.io/api/core/v1"
"kpt.dev/configsync/e2e/nomostest"
"kpt.dev/configsync/pkg/api/configmanagement"
"kpt.dev/configsync/pkg/api/configsync"
hubv1 "kpt.dev/configsync/pkg/api/hub/v1"
"kpt.dev/configsync/pkg/metadata"
"kpt.dev/configsync/pkg/reconcilermanager"
Expand Down Expand Up @@ -122,7 +123,7 @@ func ClearMembershipInfo(nt *nomostest.NT, fleetMembership, fleetProject, gkeURI

// ReconcilerPodHasFWICredsAnnotation checks whether the reconciler Pod has the
// FWI credentials annotation.
func ReconcilerPodHasFWICredsAnnotation(nt *nomostest.NT, reconcilerName, gsaEmail string) error {
func ReconcilerPodHasFWICredsAnnotation(nt *nomostest.NT, reconcilerName, gsaEmail string, auth configsync.AuthType) error {
var podList = &corev1.PodList{}
if err := nt.KubeClient.List(podList, client.InNamespace(configmanagement.ControllerNamespace), client.MatchingLabels{metadata.ReconcilerLabel: reconcilerName}); err != nil {
return err
Expand All @@ -140,7 +141,7 @@ func ReconcilerPodHasFWICredsAnnotation(nt *nomostest.NT, reconcilerName, gsaEma
if err := nt.KubeClient.Get("membership", "", membership); err != nil {
return err
}
expectedCreds, err := controllers.BuildFWICredsContent(membership.Spec.WorkloadIdentityPool, membership.Spec.IdentityProvider, gsaEmail)
expectedCreds, err := controllers.BuildFWICredsContent(membership.Spec.WorkloadIdentityPool, membership.Spec.IdentityProvider, gsaEmail, auth)
if err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion e2e/testcases/csr_auth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ const (
// references resources for tenant-a, tenant-b, and tenant-c.
// Each tenant includes a NetworkPolicy, a Role and a RoleBinding.
func csrRepo() string {
return fmt.Sprintf("https://source.developers.google.com/p/%s/r/kustomize-components", *e2e.GCPProject)
return fmt.Sprintf("%s/p/%s/r/kustomize-components", nomostesting.CSRHost, *e2e.GCPProject)
}

func gsaCSRReaderEmail() string {
Expand Down
3 changes: 2 additions & 1 deletion e2e/testcases/kcc_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,8 @@ func TestKCCResourcesOnCSR(t *testing.T) {

rs := fake.RootSyncObjectV1Beta1(configsync.RootSyncName)
nt.T.Log("sync to the kcc resources from a CSR repo")
nt.MustMergePatch(rs, fmt.Sprintf(`{"spec": {"git": {"dir": "kcc", "branch": "main", "repo": "https://source.developers.google.com/p/%s/r/configsync-kcc", "auth": "gcpserviceaccount","gcpServiceAccountEmail": "e2e-test-csr-reader@%s.iam.gserviceaccount.com", "secretRef": {"name": ""}}, "sourceFormat": "unstructured"}}`, *e2e.GCPProject, *e2e.GCPProject))
nt.MustMergePatch(rs, fmt.Sprintf(`{"spec": {"git": {"dir": "kcc", "branch": "main", "repo": "%s/p/%s/r/configsync-kcc", "auth": "gcpserviceaccount","gcpServiceAccountEmail": "e2e-test-csr-reader@%s.iam.gserviceaccount.com", "secretRef": {"name": ""}}, "sourceFormat": "unstructured"}}`,
nomostesting.CSRHost, *e2e.GCPProject, *e2e.GCPProject))

err := nt.WatchForAllSyncs(
nomostest.WithRootSha1Func(nomostest.RemoteRootRepoSha1Fn),
Expand Down
4 changes: 2 additions & 2 deletions e2e/testcases/oci_sync_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,12 +63,12 @@ const (
// privateGCRImage pulls the private OCI image by tag
// The test environment GCR is assumed to be private.
func privateGCRImage() string {
return fmt.Sprintf("gcr.io/%s/config-sync-test/kustomize-components:v1", *e2e.GCPProject)
return fmt.Sprintf("%s/%s/config-sync-test/kustomize-components:v1", nomostesting.GCRHost, *e2e.GCPProject)
}

// privateARImage() pulls the private OCI image by tag
func privateARImage() string {
return fmt.Sprintf("us-docker.pkg.dev/%s/config-sync-test-private/kustomize-components:v1", *e2e.GCPProject)
return fmt.Sprintf("%s/%s/config-sync-test-private/kustomize-components:v1", nomostesting.ARHost, *e2e.GCPProject)
}

func gsaARReaderEmail() string {
Expand Down
89 changes: 72 additions & 17 deletions e2e/testcases/workload_identity_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,10 @@ package e2e

import (
"fmt"
"strings"
"testing"

"github.com/stretchr/testify/require"
appsv1 "k8s.io/api/apps/v1"
"k8s.io/apimachinery/pkg/types"
"kpt.dev/configsync/e2e"
Expand All @@ -27,12 +29,16 @@ import (
"kpt.dev/configsync/e2e/nomostest/kustomizecomponents"
"kpt.dev/configsync/e2e/nomostest/ntopts"
nomostesting "kpt.dev/configsync/e2e/nomostest/testing"
"kpt.dev/configsync/e2e/nomostest/testpredicates"
"kpt.dev/configsync/e2e/nomostest/testutils"
"kpt.dev/configsync/e2e/nomostest/workloadidentity"
"kpt.dev/configsync/pkg/api/configsync"
"kpt.dev/configsync/pkg/api/configsync/v1beta1"
"kpt.dev/configsync/pkg/core"
"kpt.dev/configsync/pkg/declared"
"kpt.dev/configsync/pkg/kinds"
"kpt.dev/configsync/pkg/reconcilermanager"
"kpt.dev/configsync/pkg/reconcilermanager/controllers"
"kpt.dev/configsync/pkg/testing/fake"
)

Expand Down Expand Up @@ -255,27 +261,27 @@ func TestWorkloadIdentity(t *testing.T) {
v1beta1.HelmSource, tc.sourceChart, tc.sourceRepo, tc.sourceVersion, tc.gsaEmail))
}

ksaRef := types.NamespacedName{
Namespace: configsync.ControllerNamespace,
Name: core.RootReconcilerName(rs.Name),
}
nt.T.Log("Validate the GSA annotation is added to the RootSync's service account")
require.NoError(nt.T,
nt.Watcher.WatchObject(kinds.ServiceAccount(), ksaRef.Name, ksaRef.Namespace, []testpredicates.Predicate{
testpredicates.HasAnnotation(controllers.GCPSAAnnotationKey, tc.gsaEmail),
}))
if tc.fleetWITest {
nomostest.Wait(nt.T, "wait for FWI credentials to exist", nt.DefaultWaitTimeout, func() error {
return testutils.ReconcilerPodHasFWICredsAnnotation(nt, nomostest.DefaultRootReconcilerName, tc.gsaEmail)
return testutils.ReconcilerPodHasFWICredsAnnotation(nt, nomostest.DefaultRootReconcilerName, tc.gsaEmail, configsync.AuthGCPServiceAccount)
})
}
if tc.sourceType == v1beta1.HelmSource {
err := nt.WatchForAllSyncs(nomostest.WithRootSha1Func(tc.rootCommitFn),
nomostest.WithSyncDirectoryMap(map[types.NamespacedName]string{nomostest.DefaultRootRepoNamespacedName: tc.sourceChart}))
if err != nil {
nt.T.Fatal(err)
}
if err := nt.Validate(fmt.Sprintf("my-coredns-%s", tc.sourceChart), "coredns", &appsv1.Deployment{}); err != nil {
nt.T.Error(err)
}
} else {
err := nt.WatchForAllSyncs(nomostest.WithRootSha1Func(tc.rootCommitFn),
nomostest.WithSyncDirectoryMap(map[types.NamespacedName]string{nomostest.DefaultRootRepoNamespacedName: tenant}))
if err != nil {
nt.T.Fatal(err)
}
kustomizecomponents.ValidateAllTenants(nt, string(declared.RootReconciler), "../base", tenant)
validateAllResourcesReconciled(nt, tc.sourceType, tc.sourceChart, tenant, tc.rootCommitFn)

// Migrate from gcpserviceaccount to k8sserviceaccount
if strings.HasPrefix(tc.sourceRepo, nomostesting.ARHost) ||
strings.HasPrefix(tc.sourceRepo, "oci://"+nomostesting.ARHost) {
migrateFromGSAtoKSA(nt, rs, ksaRef, tc.fleetWITest)
validateAllResourcesReconciled(nt, tc.sourceType, tc.sourceChart, tenant, tc.rootCommitFn)
}
})
}
Expand All @@ -287,3 +293,52 @@ func truncateStringByLength(s string, l int) string {
}
return s
}

// migrateFromGSAtoKSA tests the scenario of migrating from impersonating a GSA
// to leveraging KSA+WI (a.k.a, BYOID/Ubermint).
func migrateFromGSAtoKSA(nt *nomostest.NT, rs *v1beta1.RootSync, ksaRef types.NamespacedName, fleetWITest bool) {
nt.T.Log("Update RootSync auth type from gcpserviceaccount to k8sserviceaccount")
oldGeneration := rs.Generation
nt.MustMergePatch(rs, fmt.Sprintf(`{"spec": {"%s": {"auth": "k8sserviceaccount", "gcpServiceAccountEmail": ""}}}`, rs.Spec.SourceType))

// Validations
nt.T.Log("Validate the GSA annotation is removed from the RootSync's service account")
require.NoError(nt.T,
nt.Watcher.WatchObject(kinds.ServiceAccount(), ksaRef.Name, ksaRef.Namespace, []testpredicates.Predicate{
testpredicates.MissingAnnotation(controllers.GCPSAAnnotationKey),
}))
if fleetWITest {
nt.T.Log("Validate the serviceaccount_impersonation_url is absent from the injected FWI credentials")
nomostest.Wait(nt.T, "wait for FWI credentials to exist", nt.DefaultWaitTimeout, func() error {
return testutils.ReconcilerPodHasFWICredsAnnotation(nt, nomostest.DefaultRootReconcilerName, "", configsync.AuthK8sServiceAccount)
})
}

nt.T.Log("Validate RootSync generation and observedGeneration is updated")
require.NoError(nt.T,
nt.Watcher.WatchObject(kinds.RootSyncV1Beta1(), rs.Name, rs.Namespace,
[]testpredicates.Predicate{
testpredicates.GenerationEquals(oldGeneration + 1),
testpredicates.HasObservedLatestGeneration(nt.Scheme),
}))
}

func validateAllResourcesReconciled(nt *nomostest.NT, sourceType v1beta1.SourceType, sourceChart, syncDir string, rootCommitFn nomostest.Sha1Func) {
if sourceType == v1beta1.HelmSource {
err := nt.WatchForAllSyncs(nomostest.WithRootSha1Func(rootCommitFn),
nomostest.WithSyncDirectoryMap(map[types.NamespacedName]string{nomostest.DefaultRootRepoNamespacedName: sourceChart}))
if err != nil {
nt.T.Fatal(err)
}
if err := nt.Validate(fmt.Sprintf("my-coredns-%s", sourceChart), "coredns", &appsv1.Deployment{}); err != nil {
nt.T.Error(err)
}
} else {
err := nt.WatchForAllSyncs(nomostest.WithRootSha1Func(rootCommitFn),
nomostest.WithSyncDirectoryMap(map[types.NamespacedName]string{nomostest.DefaultRootRepoNamespacedName: syncDir}))
if err != nil {
nt.T.Fatal(err)
}
kustomizecomponents.ValidateAllTenants(nt, string(declared.RootReconciler), "../base", syncDir)
}
}
18 changes: 12 additions & 6 deletions manifests/reposync-crd.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -171,11 +171,12 @@ spec:
properties:
auth:
description: auth specifies the type to authenticate to the Helm
repository. Must be one of token, gcpserviceaccount, gcenode
or none. The validation of this is case-sensitive. Required.
repository. Must be one of token, gcpserviceaccount, k8sserviceaccount,
gcenode or none. The validation of this is case-sensitive. Required.
enum:
- none
- gcpserviceaccount
- k8sserviceaccount
- token
- gcenode
type: string
Expand Down Expand Up @@ -287,10 +288,12 @@ spec:
auth:
description: auth is the type of secret configured for access
to the OCI package. Must be one of gcenode, gcpserviceaccount,
or none. The validation of this is case-sensitive. Required.
k8sserviceaccount, or none. The validation of this is case-sensitive.
Required.
enum:
- gcenode
- gcpserviceaccount
- k8sserviceaccount
- none
type: string
caCertSecretRef:
Expand Down Expand Up @@ -1249,11 +1252,12 @@ spec:
properties:
auth:
description: auth specifies the type to authenticate to the Helm
repository. Must be one of token, gcpserviceaccount, gcenode
or none. The validation of this is case-sensitive. Required.
repository. Must be one of token, gcpserviceaccount, k8sserviceaccount,
gcenode or none. The validation of this is case-sensitive. Required.
enum:
- none
- gcpserviceaccount
- k8sserviceaccount
- token
- gcenode
type: string
Expand Down Expand Up @@ -1365,10 +1369,12 @@ spec:
auth:
description: auth is the type of secret configured for access
to the OCI package. Must be one of gcenode, gcpserviceaccount,
or none. The validation of this is case-sensitive. Required.
k8sserviceaccount, or none. The validation of this is case-sensitive.
Required.
enum:
- gcenode
- gcpserviceaccount
- k8sserviceaccount
- none
type: string
caCertSecretRef:
Expand Down
18 changes: 12 additions & 6 deletions manifests/rootsync-crd.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -171,11 +171,12 @@ spec:
properties:
auth:
description: auth specifies the type to authenticate to the Helm
repository. Must be one of token, gcpserviceaccount, gcenode
or none. The validation of this is case-sensitive. Required.
repository. Must be one of token, gcpserviceaccount, k8sserviceaccount,
gcenode or none. The validation of this is case-sensitive. Required.
enum:
- none
- gcpserviceaccount
- k8sserviceaccount
- token
- gcenode
type: string
Expand Down Expand Up @@ -297,10 +298,12 @@ spec:
auth:
description: auth is the type of secret configured for access
to the OCI package. Must be one of gcenode, gcpserviceaccount,
or none. The validation of this is case-sensitive. Required.
k8sserviceaccount, or none. The validation of this is case-sensitive.
Required.
enum:
- gcenode
- gcpserviceaccount
- k8sserviceaccount
- none
type: string
caCertSecretRef:
Expand Down Expand Up @@ -1304,11 +1307,12 @@ spec:
properties:
auth:
description: auth specifies the type to authenticate to the Helm
repository. Must be one of token, gcpserviceaccount, gcenode
or none. The validation of this is case-sensitive. Required.
repository. Must be one of token, gcpserviceaccount, k8sserviceaccount,
gcenode or none. The validation of this is case-sensitive. Required.
enum:
- none
- gcpserviceaccount
- k8sserviceaccount
- token
- gcenode
type: string
Expand Down Expand Up @@ -1431,10 +1435,12 @@ spec:
auth:
description: auth is the type of secret configured for access
to the OCI package. Must be one of gcenode, gcpserviceaccount,
or none. The validation of this is case-sensitive. Required.
k8sserviceaccount, or none. The validation of this is case-sensitive.
Required.
enum:
- gcenode
- gcpserviceaccount
- k8sserviceaccount
- none
type: string
caCertSecretRef:
Expand Down
3 changes: 3 additions & 0 deletions pkg/api/configsync/register.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,9 @@ const (
// AuthGCPServiceAccount indicates using a GCP service account to authenticate to
// Git or OCI or Helm, when GKE Workload Identity or Fleet Workload Identity is enabled.
AuthGCPServiceAccount AuthType = "gcpserviceaccount"
// AuthK8sServiceAccount indicates using a Kubernetes service account with
// GKE or Fleet Workload Identity to authenticate to GCP services.
AuthK8sServiceAccount AuthType = "k8sserviceaccount"
)

// NamespaceStrategy specifies the strategy used by the reconciler for undeclared
Expand Down
4 changes: 2 additions & 2 deletions pkg/api/configsync/v1alpha1/helmconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,9 +95,9 @@ type HelmBase struct {
Period metav1.Duration `json:"period,omitempty"`

// auth specifies the type to authenticate to the Helm repository.
// Must be one of token, gcpserviceaccount, gcenode or none.
// Must be one of token, gcpserviceaccount, k8sserviceaccount, gcenode or none.
// The validation of this is case-sensitive. Required.
// +kubebuilder:validation:Enum=none;gcpserviceaccount;token;gcenode
// +kubebuilder:validation:Enum=none;gcpserviceaccount;k8sserviceaccount;token;gcenode
Auth configsync.AuthType `json:"auth"`

// gcpServiceAccountEmail specifies the GCP service account used to annotate
Expand Down
4 changes: 2 additions & 2 deletions pkg/api/configsync/v1alpha1/ociconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,10 @@ type Oci struct {
Period metav1.Duration `json:"period,omitempty"`

// auth is the type of secret configured for access to the OCI package.
// Must be one of gcenode, gcpserviceaccount, or none.
// Must be one of gcenode, gcpserviceaccount, k8sserviceaccount, or none.
// The validation of this is case-sensitive. Required.
//
// +kubebuilder:validation:Enum=gcenode;gcpserviceaccount;none
// +kubebuilder:validation:Enum=gcenode;gcpserviceaccount;k8sserviceaccount;none
Auth configsync.AuthType `json:"auth"`

// gcpServiceAccountEmail specifies the GCP service account used to annotate
Expand Down
Loading

0 comments on commit 2b2eed4

Please sign in to comment.