Skip to content

Commit

Permalink
Refactor git-sync env variables to constants (#904)
Browse files Browse the repository at this point in the history
  • Loading branch information
@nan-yu
nan-yu authored Sep 27, 2023
1 parent 260f13d commit 66a9608
Show file tree
Hide file tree
Showing 6 changed files with 98 additions and 67 deletions.
5 changes: 3 additions & 2 deletions e2e/testcases/no_ssl_verify_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import (
"kpt.dev/configsync/pkg/core"
"kpt.dev/configsync/pkg/kinds"
"kpt.dev/configsync/pkg/reconcilermanager"
"kpt.dev/configsync/pkg/reconcilermanager/controllers"
"kpt.dev/configsync/pkg/testing/fake"
)

Expand All @@ -39,7 +40,7 @@ func TestNoSSLVerifyV1Alpha1(t *testing.T) {
nt.T.Fatal(err)
}

key := "GIT_SSL_NO_VERIFY"
key := controllers.GitSSLNoVerify
rootReconcilerNN := types.NamespacedName{
Name: nomostest.DefaultRootReconcilerName,
Namespace: v1.NSConfigManagementSystem,
Expand Down Expand Up @@ -118,7 +119,7 @@ func TestNoSSLVerifyV1Beta1(t *testing.T) {
nt.T.Fatal(err)
}

key := "GIT_SSL_NO_VERIFY"
key := controllers.GitSSLNoVerify
rootReconcilerNN := types.NamespacedName{
Name: nomostest.DefaultRootReconcilerName,
Namespace: v1.NSConfigManagementSystem,
Expand Down
4 changes: 2 additions & 2 deletions e2e/testcases/override_git_sync_depth_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ func TestOverrideGitSyncDepthV1Alpha1(t *testing.T) {
nt.T.Fatal(err)
}

key := "GIT_SYNC_DEPTH"
key := controllers.GitSyncDepth
rootReconcilerNN := types.NamespacedName{
Name: nomostest.DefaultRootReconcilerName,
Namespace: v1.NSConfigManagementSystem,
Expand Down Expand Up @@ -140,7 +140,7 @@ func TestOverrideGitSyncDepthV1Beta1(t *testing.T) {
nt.T.Fatal(err)
}

key := "GIT_SYNC_DEPTH"
key := controllers.GitSyncDepth
rootReconcilerNN := types.NamespacedName{
Name: nomostest.DefaultRootReconcilerName,
Namespace: v1.NSConfigManagementSystem,
Expand Down
24 changes: 12 additions & 12 deletions e2e/testcases/private_cert_secret_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ func TestCACertSecretRefV1Alpha1(t *testing.T) {
nt := nomostest.New(t, nomostesting.SyncSource, ntopts.SkipNonLocalGitProvider,
ntopts.NamespaceRepo(backendNamespace, configsync.RepoSyncName))

key := "GIT_SSL_CAINFO"
key := controllers.GitSSLCAInfo
caCertSecret := "git-cert-pub"
caCertPath := "/etc/ca-cert/cert"
var err error
Expand All @@ -87,7 +87,7 @@ func TestCACertSecretRefV1Alpha1(t *testing.T) {
nt.MustMergePatch(rootSync, syncURLHTTPSPatch(rootSyncHTTPS))
// RootSync should fail without caCertSecret
nt.WaitForRootSyncSourceError(configsync.RootSyncName, status.SourceErrorCode, "server certificate verification failed")
err = nt.Validate(nomostest.DefaultRootReconcilerName, v1.NSConfigManagementSystem, &appsv1.Deployment{}, testpredicates.DeploymentHasEnvVar(reconcilermanager.GitSync, "GIT_SYNC_REPO", rootSyncHTTPS))
err = nt.Validate(nomostest.DefaultRootReconcilerName, v1.NSConfigManagementSystem, &appsv1.Deployment{}, testpredicates.DeploymentHasEnvVar(reconcilermanager.GitSync, controllers.GitSyncRepo, rootSyncHTTPS))
if err != nil {
nt.T.Fatal(err)
}
Expand All @@ -112,7 +112,7 @@ func TestCACertSecretRefV1Alpha1(t *testing.T) {
nt.Must(nt.RootRepos[configsync.RootSyncName].CommitAndPush("Update backend RepoSync use HTTPS"))
// RepoSync should fail without caCertSecret
nt.WaitForRepoSyncSourceError(backendNamespace, configsync.RepoSyncName, status.SourceErrorCode, "server certificate verification failed")
err = nt.Validate(reconcilerName, v1.NSConfigManagementSystem, &appsv1.Deployment{}, testpredicates.DeploymentHasEnvVar(reconcilermanager.GitSync, "GIT_SYNC_REPO", repoSyncHTTPS))
err = nt.Validate(reconcilerName, v1.NSConfigManagementSystem, &appsv1.Deployment{}, testpredicates.DeploymentHasEnvVar(reconcilermanager.GitSync, controllers.GitSyncRepo, repoSyncHTTPS))
if err != nil {
nt.T.Fatal(err)
}
Expand Down Expand Up @@ -144,7 +144,7 @@ func TestCACertSecretRefV1Alpha1(t *testing.T) {
if err := nt.WatchForAllSyncs(); err != nil {
nt.T.Fatal(err)
}
err = nt.Validate(nomostest.DefaultRootReconcilerName, v1.NSConfigManagementSystem, &appsv1.Deployment{}, testpredicates.DeploymentHasEnvVar(reconcilermanager.GitSync, "GIT_SYNC_REPO", rootSyncSSHURL))
err = nt.Validate(nomostest.DefaultRootReconcilerName, v1.NSConfigManagementSystem, &appsv1.Deployment{}, testpredicates.DeploymentHasEnvVar(reconcilermanager.GitSync, controllers.GitSyncRepo, rootSyncSSHURL))
if err != nil {
nt.T.Fatal(err)
}
Expand All @@ -170,7 +170,7 @@ func TestCACertSecretRefV1Alpha1(t *testing.T) {
if err := nt.WatchForAllSyncs(); err != nil {
nt.T.Fatal(err)
}
err = nt.Validate(reconcilerName, v1.NSConfigManagementSystem, &appsv1.Deployment{}, testpredicates.DeploymentHasEnvVar(reconcilermanager.GitSync, "GIT_SYNC_REPO", repoSyncSSHURL))
err = nt.Validate(reconcilerName, v1.NSConfigManagementSystem, &appsv1.Deployment{}, testpredicates.DeploymentHasEnvVar(reconcilermanager.GitSync, controllers.GitSyncRepo, repoSyncSSHURL))
if err != nil {
nt.T.Fatal(err)
}
Expand All @@ -180,7 +180,7 @@ func TestCACertSecretRefV1Beta1(t *testing.T) {
nt := nomostest.New(t, nomostesting.SyncSource, ntopts.SkipNonLocalGitProvider,
ntopts.NamespaceRepo(backendNamespace, configsync.RepoSyncName))

key := "GIT_SSL_CAINFO"
key := controllers.GitSSLCAInfo
caCertSecret := "git-cert-pub"
caCertPath := "/etc/ca-cert/cert"
var err error
Expand All @@ -205,7 +205,7 @@ func TestCACertSecretRefV1Beta1(t *testing.T) {
nt.MustMergePatch(rootSync, syncURLHTTPSPatch(rootSyncHTTPS))
// RootSync should fail without caCertSecret
nt.WaitForRootSyncSourceError(configsync.RootSyncName, status.SourceErrorCode, "server certificate verification failed")
err = nt.Validate(nomostest.DefaultRootReconcilerName, v1.NSConfigManagementSystem, &appsv1.Deployment{}, testpredicates.DeploymentHasEnvVar(reconcilermanager.GitSync, "GIT_SYNC_REPO", rootSyncHTTPS))
err = nt.Validate(nomostest.DefaultRootReconcilerName, v1.NSConfigManagementSystem, &appsv1.Deployment{}, testpredicates.DeploymentHasEnvVar(reconcilermanager.GitSync, controllers.GitSyncRepo, rootSyncHTTPS))
if err != nil {
nt.T.Fatal(err)
}
Expand All @@ -229,7 +229,7 @@ func TestCACertSecretRefV1Beta1(t *testing.T) {
nt.Must(nt.RootRepos[configsync.RootSyncName].CommitAndPush("Update backend RepoSync use HTTPS"))
// RepoSync should fail without caCertSecret
nt.WaitForRepoSyncSourceError(backendNamespace, configsync.RepoSyncName, status.SourceErrorCode, "server certificate verification failed")
err = nt.Validate(reconcilerName, v1.NSConfigManagementSystem, &appsv1.Deployment{}, testpredicates.DeploymentHasEnvVar(reconcilermanager.GitSync, "GIT_SYNC_REPO", repoSyncHTTPS))
err = nt.Validate(reconcilerName, v1.NSConfigManagementSystem, &appsv1.Deployment{}, testpredicates.DeploymentHasEnvVar(reconcilermanager.GitSync, controllers.GitSyncRepo, repoSyncHTTPS))
if err != nil {
nt.T.Fatal(err)
}
Expand Down Expand Up @@ -267,7 +267,7 @@ func TestCACertSecretRefV1Beta1(t *testing.T) {
if err := nt.WatchForAllSyncs(); err != nil {
nt.T.Fatal(err)
}
err = nt.Validate(nomostest.DefaultRootReconcilerName, v1.NSConfigManagementSystem, &appsv1.Deployment{}, testpredicates.DeploymentHasEnvVar(reconcilermanager.GitSync, "GIT_SYNC_REPO", rootSyncSSHURL))
err = nt.Validate(nomostest.DefaultRootReconcilerName, v1.NSConfigManagementSystem, &appsv1.Deployment{}, testpredicates.DeploymentHasEnvVar(reconcilermanager.GitSync, controllers.GitSyncRepo, rootSyncSSHURL))
if err != nil {
nt.T.Fatal(err)
}
Expand All @@ -293,7 +293,7 @@ func TestCACertSecretRefV1Beta1(t *testing.T) {
if err := nt.WatchForAllSyncs(); err != nil {
nt.T.Fatal(err)
}
err = nt.Validate(reconcilerName, v1.NSConfigManagementSystem, &appsv1.Deployment{}, testpredicates.DeploymentHasEnvVar(reconcilermanager.GitSync, "GIT_SYNC_REPO", repoSyncSSHURL))
err = nt.Validate(reconcilerName, v1.NSConfigManagementSystem, &appsv1.Deployment{}, testpredicates.DeploymentHasEnvVar(reconcilermanager.GitSync, controllers.GitSyncRepo, repoSyncSSHURL))
if err != nil {
nt.T.Fatal(err)
}
Expand All @@ -303,7 +303,7 @@ func TestCACertSecretWatch(t *testing.T) {
nt := nomostest.New(t, nomostesting.SyncSource, ntopts.SkipNonLocalGitProvider,
ntopts.NamespaceRepo(backendNamespace, configsync.RepoSyncName))

key := "GIT_SSL_CAINFO"
key := controllers.GitSSLCAInfo
caCertSecret := "git-cert-pub"
caCertPath := "/etc/ca-cert/cert"
var err error
Expand Down Expand Up @@ -375,7 +375,7 @@ func TestCACertSecretWatch(t *testing.T) {
if err := nt.WatchForAllSyncs(); err != nil {
nt.T.Fatal(err)
}
err = nt.Validate(reconcilerName, v1.NSConfigManagementSystem, &appsv1.Deployment{}, testpredicates.DeploymentHasEnvVar(reconcilermanager.GitSync, "GIT_SYNC_REPO", repoSyncSSHURL))
err = nt.Validate(reconcilerName, v1.NSConfigManagementSystem, &appsv1.Deployment{}, testpredicates.DeploymentHasEnvVar(reconcilermanager.GitSync, controllers.GitSyncRepo, repoSyncSSHURL))
if err != nil {
nt.T.Fatal(err)
}
Expand Down
72 changes: 51 additions & 21 deletions pkg/reconcilermanager/controllers/gitsync_env.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,39 @@ import (

const (
// git-sync container specific environment variables.
gitSyncName = "GIT_SYNC_USERNAME"
gitSyncPassword = "GIT_SYNC_PASSWORD"

// gitSyncUsername represents the environment variable key for specifying the username to use for git auth.
gitSyncUsername = "GIT_SYNC_USERNAME"
// gitSyncPassword represents the environment variable key for specifying the password to use for git auth.
gitSyncPassword = "GIT_SYNC_PASSWORD"
// gitSyncHTTPSProxy represents the environment variable key for setting `HTTPS_PROXY` in git-sync.
gitSyncHTTPSProxy = "HTTPS_PROXY"
// GitSyncRepo represents the environment variable key for specifying the Git repository to sync.
GitSyncRepo = "GIT_SYNC_REPO"
// gitSyncBranch represents the environment variable key for specifying the Git branch to sync.
gitSyncBranch = "GIT_SYNC_BRANCH"
// gitSyncRev represents the environment variable key for specifying the Git revision to sync.
gitSyncRev = "GIT_SYNC_REV"
// GitSyncDepth represents the environment variable key for setting the depth of the Git clone, truncating history to a specific number of commits.
GitSyncDepth = "GIT_SYNC_DEPTH"
// gitSyncPeriod represents the environment variable key for specifying the sync interval duration.
gitSyncPeriod = "GIT_SYNC_WAIT"

// gitSyncSSH represents the environment variable key for specifying the SSH key to use.
gitSyncSSH = "GIT_SYNC_SSH"
// gitSyncAskpassURL represents the environment variable key for the URL used to query git credentials.
gitSyncAskpassURL = "GIT_ASKPASS_URL"

// gitSyncCookieFile represents the environment variable key for specifying the use of a git cookiefile.
gitSyncCookieFile = "GIT_COOKIE_FILE"

// GitSSLCAInfo represents the environment variable key for SSL certificates.
GitSSLCAInfo = "GIT_SSL_CAINFO"

// gitSyncKnownHosts represents the environment variable key for GIT_KNOWN_HOSTS.
gitSyncKnownHosts = "GIT_KNOWN_HOSTS"
// GitSSLNoVerify represents the environment variable key for GIT_SSL_NO_VERIFY.
GitSSLNoVerify = "GIT_SSL_NO_VERIFY"

// DefaultSyncRev is the default git revision.
DefaultSyncRev = "HEAD"
Expand Down Expand Up @@ -69,7 +99,7 @@ type options struct {

// gitSyncTokenAuthEnv returns environment variables for git-sync container for 'token' Auth.
func gitSyncTokenAuthEnv(secretRef string) []corev1.EnvVar {
gitSyncUsername := &corev1.EnvVarSource{
username := &corev1.EnvVarSource{
SecretKeyRef: &corev1.SecretKeySelector{
LocalObjectReference: corev1.LocalObjectReference{
Name: secretRef,
Expand All @@ -78,7 +108,7 @@ func gitSyncTokenAuthEnv(secretRef string) []corev1.EnvVar {
},
}

gitSyncPswd := &corev1.EnvVarSource{
passwd := &corev1.EnvVarSource{
SecretKeyRef: &corev1.SecretKeySelector{
LocalObjectReference: corev1.LocalObjectReference{
Name: secretRef,
Expand All @@ -89,12 +119,12 @@ func gitSyncTokenAuthEnv(secretRef string) []corev1.EnvVar {

return []corev1.EnvVar{
{
Name: gitSyncName,
ValueFrom: gitSyncUsername,
Name: gitSyncUsername,
ValueFrom: username,
},
{
Name: gitSyncPassword,
ValueFrom: gitSyncPswd,
ValueFrom: passwd,
},
}
}
Expand Down Expand Up @@ -131,31 +161,31 @@ func useCACert(caCertSecretRef string) bool {
func gitSyncEnvs(_ context.Context, opts options) []corev1.EnvVar {
var result []corev1.EnvVar
result = append(result, corev1.EnvVar{
Name: "GIT_SYNC_REPO",
Name: GitSyncRepo,
Value: opts.repo,
})
// disable known_hosts checking because it provides no benefit for our use case.
result = append(result, corev1.EnvVar{
Name: "GIT_KNOWN_HOSTS",
Name: gitSyncKnownHosts,
Value: "false",
})
if opts.noSSLVerify {
result = append(result, corev1.EnvVar{
Name: "GIT_SSL_NO_VERIFY",
Name: GitSSLNoVerify,
Value: "true",
})
}
if useCACert(opts.caCertSecretRef) {
result = append(result, corev1.EnvVar{
Name: "GIT_SSL_CAINFO",
Name: GitSSLCAInfo,
Value: fmt.Sprintf("%s/%s", CACertPath, CACertSecretKey),
})
}
if opts.depth != nil && *opts.depth >= 0 {
// git-sync would do a shallow clone if *opts.depth > 0;
// git-sync would do a full clone if *opts.depth == 0.
result = append(result, corev1.EnvVar{
Name: "GIT_SYNC_DEPTH",
Name: GitSyncDepth,
Value: strconv.FormatInt(*opts.depth, 10),
})
} else {
Expand All @@ -170,56 +200,56 @@ func gitSyncEnvs(_ context.Context, opts options) []corev1.EnvVar {
// See b/175088702 and b/158988143
if opts.ref == "" || opts.ref == DefaultSyncRev {
result = append(result, corev1.EnvVar{
Name: "GIT_SYNC_DEPTH",
Name: GitSyncDepth,
Value: SyncDepthNoRev,
})
} else {
result = append(result, corev1.EnvVar{
Name: "GIT_SYNC_DEPTH",
Name: GitSyncDepth,
Value: SyncDepthRev,
})
}
}
result = append(result, corev1.EnvVar{
Name: "GIT_SYNC_WAIT",
Name: gitSyncPeriod,
Value: fmt.Sprintf("%f", opts.period),
})
// When branch and ref not set in RootSync/RepoSync then dont set GIT_SYNC_BRANCH
// and GIT_SYNC_REV, git-sync will use the default values for them.
if opts.branch != "" {
result = append(result, corev1.EnvVar{
Name: "GIT_SYNC_BRANCH",
Name: gitSyncBranch,
Value: opts.branch,
})
}
if opts.ref != "" {
result = append(result, corev1.EnvVar{
Name: "GIT_SYNC_REV",
Name: gitSyncRev,
Value: opts.ref,
})
}
switch opts.secretType {
case configsync.AuthGCENode, configsync.AuthGCPServiceAccount:
result = append(result, corev1.EnvVar{
Name: "GIT_ASKPASS_URL",
Name: gitSyncAskpassURL,
Value: gceNodeAskpassURL,
})
case configsync.AuthSSH:
result = append(result, corev1.EnvVar{
Name: "GIT_SYNC_SSH",
Name: gitSyncSSH,
Value: "true",
})
case configsync.AuthCookieFile:
result = append(result, corev1.EnvVar{
Name: "GIT_COOKIE_FILE",
Name: gitSyncCookieFile,
Value: "true",
})

fallthrough
case GitSecretConfigKeyToken, "", configsync.AuthNone:
if opts.proxy != "" {
result = append(result, corev1.EnvVar{
Name: "HTTPS_PROXY",
Name: gitSyncHTTPSProxy,
Value: opts.proxy,
})
}
Expand Down
Loading

0 comments on commit 66a9608

Please sign in to comment.