Revert "Merge pull request openshift#1006 from abhinavdahiya/user-pro…

…vided-sa-signing-key" This reverts commit 00b787c, reversing changes made to 98d8ee4.
abhinavdahiya · Nov 23, 2020 · 2e34ad9 · 2e34ad9
1 parent fc9e9d5
commit 2e34ad9
Show file tree

Hide file tree

Showing 4 changed files with 0 additions and 284 deletions.
diff --git a/bindata/bootkube/config/bootstrap-config-overrides.yaml b/bindata/bootkube/config/bootstrap-config-overrides.yaml
@@ -72,13 +72,6 @@ apiServerArguments:
     - /etc/kubernetes/secrets/aggregator-signer.crt
   service-account-key-file:
     - /etc/kubernetes/secrets/service-account.pub
-  {{- if .UserProvidedBoundSASigningKey}}
-    - /etc/kubernetes/secrets/bound-service-account-signing-key.pub
-  {{- end}}
-  service-account-issuer: {{if .ServiceAccountIssuer}}
-    - {{.ServiceAccountIssuer}}{{end}}
-  service-account-signing-key-file: {{if .UserProvidedBoundSASigningKey}}
-    - /etc/kubernetes/secrets/bound-service-account-signing-key.key{{end}}
   tls-cert-file:
     - /etc/kubernetes/secrets/kube-apiserver-service-network-server.crt
   tls-private-key-file:

diff --git a/bindata/bootkube/manifests/secret-bound-sa-token-signing-key.yaml b/bindata/bootkube/manifests/secret-bound-sa-token-signing-key.yaml
diff --git a/pkg/cmd/render/render.go b/pkg/cmd/render/render.go
@@ -7,7 +7,6 @@ import (
 	"fmt"
 	"io/ioutil"
 	"net"
-	"os"
 	"path/filepath"
 
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
@@ -39,7 +38,6 @@ type renderOpts struct {
 	etcdServerURLs    []string
 	etcdServingCA     string
 	clusterConfigFile string
-	clusterAuthFile   string
 }
 
 // NewRenderCommand creates a render command.
@@ -81,7 +79,6 @@ func (r *renderOpts) AddFlags(fs *pflag.FlagSet) {
 	fs.StringArrayVar(&r.etcdServerURLs, "manifest-etcd-server-urls", r.etcdServerURLs, "The etcd server URL, comma separated.")
 	fs.StringVar(&r.etcdServingCA, "manifest-etcd-serving-ca", r.etcdServingCA, "The etcd serving CA.")
 	fs.StringVar(&r.clusterConfigFile, "cluster-config-file", r.clusterConfigFile, "Openshift Cluster API Config file.")
-	fs.StringVar(&r.clusterAuthFile, "cluster-auth-file", r.clusterAuthFile, "Openshift Cluster Authentication API Config file.")
 }
 
 // Validate verifies the inputs.
@@ -145,10 +142,6 @@ type TemplateData struct {
 
 	// BindNetwork is the network (tcp4 or tcp6) to bind to
 	BindNetwork string
-
-	ServiceAccountIssuer string
-
-	UserProvidedBoundSASigningKey bool
 }
 
 // Run contains the logic of the render command.
@@ -169,22 +162,6 @@ func (r *renderOpts) Run() error {
 			return fmt.Errorf("unable to parse restricted CIDRs from config %q: %v", r.clusterConfigFile, err)
 		}
 	}
-	if len(r.clusterAuthFile) > 0 {
-		clusterAuthFileData, err := ioutil.ReadFile(r.clusterAuthFile)
-		if err != nil && !os.IsNotExist(err) {
-			return fmt.Errorf("failed to load authentication config: %v", err)
-		}
-		if len(clusterAuthFileData) > 0 {
-			if err := discoverServiceAccountIssuer(clusterAuthFileData, &renderConfig); err != nil {
-				return fmt.Errorf("unable to parse service-account issuers from config %q: %v", r.clusterAuthFile, err)
-			}
-		}
-	}
-	if _, err := os.Stat(filepath.Join(r.generic.AssetInputDir, "bound-service-account-signing-key.key")); err == nil {
-		if _, err := os.Stat(filepath.Join(r.generic.AssetInputDir, "bound-service-account-signing-key.pub")); err == nil {
-			renderConfig.UserProvidedBoundSASigningKey = true
-		}
-	}
 	if len(renderConfig.ClusterCIDR) > 0 {
 		anyIPv4 := false
 		for _, cidr := range renderConfig.ClusterCIDR {
@@ -306,27 +283,6 @@ func mustReadTemplateFile(fname string) genericrenderoptions.Template {
 	return genericrenderoptions.Template{FileName: fname, Content: bs}
 }
 
-func discoverServiceAccountIssuer(clusterAuthFileData []byte, renderConfig *TemplateData) error {
-	configJson, err := yaml.YAMLToJSON(clusterAuthFileData)
-	if err != nil {
-		return err
-	}
-	clusterConfigObj, err := runtime.Decode(unstructured.UnstructuredJSONScheme, configJson)
-	if err != nil {
-		return err
-	}
-	clusterConfig, ok := clusterConfigObj.(*unstructured.Unstructured)
-	if !ok {
-		return fmt.Errorf("unexpected object in %t", clusterConfigObj)
-	}
-	issuer, found, err := unstructured.NestedString(
-		clusterConfig.Object, "spec", "serviceAccountIssuer")
-	if found && err == nil {
-		renderConfig.ServiceAccountIssuer = issuer
-	}
-	return err
-}
-
 func discoverCIDRs(clusterConfigFileData []byte, renderConfig *TemplateData) error {
 	if err := discoverCIDRsFromNetwork(clusterConfigFileData, renderConfig); err != nil {
 		if err = discoverCIDRsFromClusterAPI(clusterConfigFileData, renderConfig); err != nil {

diff --git a/pkg/cmd/render/render_test.go b/pkg/cmd/render/render_test.go
@@ -129,43 +129,6 @@ func TestDiscoverCIDRsFromClusterAPI(t *testing.T) {
 	}
 }
 
-func TestDiscoverServiceAccountIssuer(t *testing.T) {
-	tests := []struct {
-		config string
-
-		issuer string
-	}{{
-		config: `apiVersion: config.openshift.io/v1
-kind: Authentication
-metadata:
-  name: cluster
-spec: {}`,
-	}, {
-		config: `apiVersion: config.openshift.io/v1
-kind: Authentication
-metadata:
-  name: cluster
-spec:
-  serviceAccountIssuer: https://test.dummy.url`,
-		issuer: "https://test.dummy.url",
-	}}
-	for _, test := range tests {
-		t.Run("", func(t *testing.T) {
-			renderConfig := TemplateData{
-				LockHostPath:   "",
-				EtcdServerURLs: []string{""},
-				EtcdServingCA:  "",
-			}
-			if err := discoverServiceAccountIssuer([]byte(test.config), &renderConfig); err != nil {
-				t.Fatalf("failed to discoverServiceAccountIssuer: %v", err)
-			}
-			if !reflect.DeepEqual(renderConfig.ServiceAccountIssuer, test.issuer) {
-				t.Fatalf("Got: %s, expected: %v", renderConfig.ServiceAccountIssuer, test.issuer)
-			}
-		})
-	}
-}
-
 func TestDiscoverCIDRs(t *testing.T) {
 	testCase := []struct {
 		config []byte
@@ -313,190 +276,6 @@ func TestRenderCommand(t *testing.T) {
 				return nil
 			},
 		},
-		{
-			name: "scenario 4 checks service account issuer when authentication no exists",
-			args: []string{
-				"--asset-input-dir=" + assetsInputDir,
-				"--templates-input-dir=" + templateDir,
-				"--cluster-auth-file=" + filepath.Join(assetsInputDir, "authentication.yaml"),
-				"--asset-output-dir=",
-				"--config-output-file=",
-			},
-			testFunction: func(cfg *kubecontrolplanev1.KubeAPIServerConfig) error {
-				if len(cfg.APIServerArguments["service-account-issuer"]) > 0 {
-					return fmt.Errorf("expected the service-account-issuer to be empty, but it was %s", cfg.APIServerArguments["service-account-issuer"])
-				}
-				return nil
-			},
-		},
-		{
-			name: "scenario 5 checks service account issuer when authentication exists but empty",
-			args: []string{
-				"--asset-input-dir=" + assetsInputDir,
-				"--templates-input-dir=" + templateDir,
-				"--cluster-auth-file=" + filepath.Join(assetsInputDir, "authentication.yaml"),
-				"--asset-output-dir=",
-				"--config-output-file=",
-			},
-			setupFunction: func() error {
-				data := ``
-				return ioutil.WriteFile(filepath.Join(assetsInputDir, "authentication.yaml"), []byte(data), 0644)
-			},
-			testFunction: func(cfg *kubecontrolplanev1.KubeAPIServerConfig) error {
-				if len(cfg.APIServerArguments["service-account-issuer"]) > 0 {
-					return fmt.Errorf("expected the service-account-issuer to be empty, but it was %s", cfg.APIServerArguments["service-account-issuer"])
-				}
-				return nil
-			},
-		},
-		{
-			name: "scenario 6 checks service account issuer when authentication exists but empty spec",
-			args: []string{
-				"--asset-input-dir=" + assetsInputDir,
-				"--templates-input-dir=" + templateDir,
-				"--cluster-auth-file=" + filepath.Join(assetsInputDir, "authentication.yaml"),
-				"--asset-output-dir=",
-				"--config-output-file=",
-			},
-			setupFunction: func() error {
-				data := `apiVersion: config.openshift.io/v1
-kind: Authentication
-metadata:
-  name: cluster
-spec: {}`
-				return ioutil.WriteFile(filepath.Join(assetsInputDir, "authentication.yaml"), []byte(data), 0644)
-			},
-			testFunction: func(cfg *kubecontrolplanev1.KubeAPIServerConfig) error {
-				if len(cfg.APIServerArguments["service-account-issuer"]) > 0 {
-					return fmt.Errorf("expected the service-account-issuer to be empty, but it was %s", cfg.APIServerArguments["service-account-issuer"])
-				}
-				return nil
-			},
-		},
-		{
-			name: "scenario 7 checks service account issuer when authentication spec has issuer set",
-			args: []string{
-				"--asset-input-dir=" + assetsInputDir,
-				"--templates-input-dir=" + templateDir,
-				"--cluster-auth-file=" + filepath.Join(assetsInputDir, "authentication.yaml"),
-				"--asset-output-dir=",
-				"--config-output-file=",
-			},
-			setupFunction: func() error {
-				data := `apiVersion: config.openshift.io/v1
-kind: Authentication
-metadata:
-  name: cluster
-spec:
-  serviceAccountIssuer: https://test.dummy.url`
-				return ioutil.WriteFile(filepath.Join(assetsInputDir, "authentication.yaml"), []byte(data), 0644)
-			},
-			testFunction: func(cfg *kubecontrolplanev1.KubeAPIServerConfig) error {
-				if len(cfg.APIServerArguments["service-account-issuer"]) == 0 {
-					return fmt.Errorf("expected the service-account-issuer to be set, but it was empty")
-				}
-				if !reflect.DeepEqual(cfg.APIServerArguments["service-account-issuer"], kubecontrolplanev1.Arguments([]string{"https://test.dummy.url"})) {
-					return fmt.Errorf("expected the service-account-issuer to be [ https://test.dummy.url ], but it was %s", cfg.APIServerArguments["service-account-issuer"])
-				}
-				return nil
-			},
-		},
-		{
-			name: "scenario 8 no user provided bound-sa-signing-keys",
-			args: []string{
-				"--asset-input-dir=" + assetsInputDir,
-				"--templates-input-dir=" + templateDir,
-				"--asset-output-dir=",
-				"--config-output-file=",
-			},
-			testFunction: func(cfg *kubecontrolplanev1.KubeAPIServerConfig) error {
-				if len(cfg.APIServerArguments["service-account-signing-key-file"]) > 0 {
-					return fmt.Errorf("expected the service-account-issuer to be empty, but it was %s", cfg.APIServerArguments["service-account-signing-key-file"])
-				}
-				return nil
-			},
-		},
-		{
-			name: "scenario 9 user provided bound-sa-signing-key only no public part",
-			args: []string{
-				"--asset-input-dir=" + filepath.Join(assetsInputDir, "0"),
-				"--templates-input-dir=" + templateDir,
-				"--asset-output-dir=",
-				"--config-output-file=",
-			},
-			setupFunction: func() error {
-				data := `DUMMY DATA`
-				if err := os.Mkdir(filepath.Join(assetsInputDir, "0"), 0700); err != nil {
-					return err
-				}
-				return ioutil.WriteFile(filepath.Join(assetsInputDir, "0", "bound-service-account-signing-key.key"), []byte(data), 0644)
-			},
-			testFunction: func(cfg *kubecontrolplanev1.KubeAPIServerConfig) error {
-				if len(cfg.APIServerArguments["service-account-signing-key-file"]) > 0 {
-					return fmt.Errorf("expected the service-account-issuer to be empty, but it was %s", cfg.APIServerArguments["service-account-signing-key-file"])
-				}
-				return nil
-			},
-		},
-		{
-			name: "scenario 10 user provided bound-sa-signing-key only public part",
-			args: []string{
-				"--asset-input-dir=" + filepath.Join(assetsInputDir, "1"),
-				"--templates-input-dir=" + templateDir,
-				"--asset-output-dir=",
-				"--config-output-file=",
-			},
-			setupFunction: func() error {
-				data := `DUMMY DATA`
-				if err := os.Mkdir(filepath.Join(assetsInputDir, "1"), 0700); err != nil {
-					return err
-				}
-				return ioutil.WriteFile(filepath.Join(assetsInputDir, "1", "bound-service-account-signing-key.pub"), []byte(data), 0644)
-			},
-			testFunction: func(cfg *kubecontrolplanev1.KubeAPIServerConfig) error {
-				if len(cfg.APIServerArguments["service-account-signing-key-file"]) > 0 {
-					return fmt.Errorf("expected the service-account-issuer to be empty, but it was %s", cfg.APIServerArguments["service-account-signing-key-file"])
-				}
-				return nil
-			},
-		},
-		{
-			name: "scenario 11 user provided bound-sa-signing-key and public part",
-			args: []string{
-				"--asset-input-dir=" + filepath.Join(assetsInputDir, "2"),
-				"--templates-input-dir=" + templateDir,
-				"--asset-output-dir=",
-				"--config-output-file=",
-			},
-			setupFunction: func() error {
-				data := `DUMMY DATA`
-				if err := os.Mkdir(filepath.Join(assetsInputDir, "2"), 0700); err != nil {
-					return err
-				}
-				if err := ioutil.WriteFile(filepath.Join(assetsInputDir, "2", "bound-service-account-signing-key.key"), []byte(data), 0644); err != nil {
-					return err
-				}
-				if err := ioutil.WriteFile(filepath.Join(assetsInputDir, "2", "bound-service-account-signing-key.pub"), []byte(data), 0644); err != nil {
-					return err
-				}
-				return nil
-			},
-			testFunction: func(cfg *kubecontrolplanev1.KubeAPIServerConfig) error {
-				if len(cfg.APIServerArguments["service-account-signing-key-file"]) == 0 {
-					return fmt.Errorf("expected the service-account-issuer to be set, but it was empty")
-				}
-				if !reflect.DeepEqual(cfg.APIServerArguments["service-account-signing-key-file"], kubecontrolplanev1.Arguments([]string{"/etc/kubernetes/secrets/bound-service-account-signing-key.key"})) {
-					return fmt.Errorf("expected the service-account-issuer to be [ /etc/kubernetes/secrets/bound-service-account-signing-key.key ], but it was %s", cfg.APIServerArguments["service-account-signing-key-file"])
-				}
-				if !reflect.DeepEqual(
-					cfg.APIServerArguments["service-account-key-file"],
-					kubecontrolplanev1.Arguments([]string{"/etc/kubernetes/secrets/service-account.pub", "/etc/kubernetes/secrets/bound-service-account-signing-key.pub"}),
-				) {
-					return fmt.Errorf("expected the service-account-issuer to be [ /etc/kubernetes/secrets/service-account.pub , /etc/kubernetes/secrets/bound-service-account-signing-key.pub ], but it was %s", cfg.APIServerArguments["service-account-key-file"])
-				}
-				return nil
-			},
-		},
 	}
 
 	for _, test := range tests {