Skip to content

Commit

Permalink
Refactor registration
Browse files Browse the repository at this point in the history
Signed-off-by: Jian Qiu <jqiu@redhat.com>
  • Loading branch information
qiujian16 committed Jun 20, 2024
1 parent 4a32909 commit 5eed113
Show file tree
Hide file tree
Showing 21 changed files with 871 additions and 848 deletions.
32 changes: 4 additions & 28 deletions pkg/common/options/agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,7 @@ import (
"k8s.io/apimachinery/pkg/util/uuid"
"k8s.io/client-go/rest"
"k8s.io/client-go/tools/clientcmd"
"k8s.io/klog/v2"

"open-cluster-management.io/ocm/pkg/registration/clientcert"
"open-cluster-management.io/ocm/pkg/registration/spoke/registration"
"open-cluster-management.io/ocm/pkg/registration/register"
)

const (
Expand Down Expand Up @@ -96,7 +93,7 @@ func (o *AgentOptions) Validate() error {
// Complete fills in missing values.
func (o *AgentOptions) Complete() error {
if len(o.HubKubeconfigFile) == 0 {
o.HubKubeconfigFile = path.Join(o.HubKubeconfigDir, clientcert.KubeconfigFile)
o.HubKubeconfigFile = path.Join(o.HubKubeconfigDir, register.KubeconfigFile)
}

// load or generate cluster/agent names
Expand All @@ -122,29 +119,16 @@ func (o *AgentOptions) getOrGenerateClusterAgentID() (string, string) {
if len(o.SpokeClusterName) > 0 && len(o.AgentID) > 0 {
return o.SpokeClusterName, o.AgentID
}
// try to load cluster/agent name from tls certification
var clusterNameInCert, agentNameInCert string
certPath := path.Join(o.HubKubeconfigDir, clientcert.TLSCertFile)
certData, certErr := os.ReadFile(path.Clean(certPath))
if certErr == nil {
clusterNameInCert, agentNameInCert, _ = registration.GetClusterAgentNamesFromCertificate(certData)
}

clusterName := o.SpokeClusterName
// if cluster name is not specified with input argument, try to load it from file
if clusterName == "" {
// TODO, read cluster name from openshift struct if the spoke agent is running in an openshift cluster

// and then load the cluster name from the mounted secret
clusterNameFilePath := path.Join(o.HubKubeconfigDir, clientcert.ClusterNameFile)
clusterNameFilePath := path.Join(o.HubKubeconfigDir, register.ClusterNameFile)
clusterNameBytes, err := os.ReadFile(path.Clean(clusterNameFilePath))
switch {
case len(clusterNameInCert) > 0:
// use cluster name loaded from the tls certification
clusterName = clusterNameInCert
if clusterNameInCert != string(clusterNameBytes) {
klog.Warningf("Use cluster name %q in certification instead of %q in the mounted secret", clusterNameInCert, string(clusterNameBytes))
}
case err == nil:
// use cluster name load from the mounted secret
clusterName = string(clusterNameBytes)
Expand All @@ -157,17 +141,9 @@ func (o *AgentOptions) getOrGenerateClusterAgentID() (string, string) {
agentID := o.AgentID
// try to load agent name from the mounted secret
if len(agentID) == 0 {
agentIDFilePath := path.Join(o.HubKubeconfigDir, clientcert.AgentNameFile)
agentIDFilePath := path.Join(o.HubKubeconfigDir, register.AgentNameFile)
agentIDBytes, err := os.ReadFile(path.Clean(agentIDFilePath))
switch {
case len(agentNameInCert) > 0:
// use agent name loaded from the tls certification
agentID = agentNameInCert
if agentNameInCert != agentID {
klog.Warningf(
"Use agent name %q in certification instead of %q in the mounted secret",
agentNameInCert, agentID)
}
case err == nil:
// use agent name loaded from the mounted secret
agentID = string(agentIDBytes)
Expand Down
89 changes: 89 additions & 0 deletions pkg/registration/register/common.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
package register

import (
"fmt"
"reflect"

clientcmdapi "k8s.io/client-go/tools/clientcmd/api"
)

// BaseKubeConfigFromBootStrap builds kubeconfig from bootstrap without authInfo configurations
func BaseKubeConfigFromBootStrap(bootstrapConfig *clientcmdapi.Config) (*clientcmdapi.Config, error) {
kubeConfigCtx, cluster, err := currentKubeConfigCluster(bootstrapConfig)
if err != nil {
return nil, err
}

// Build kubeconfig.
kubeconfig := &clientcmdapi.Config{
// Define a cluster stanza based on the bootstrap kubeconfig.
Clusters: map[string]*clientcmdapi.Cluster{
kubeConfigCtx.Cluster: {
Server: cluster.Server,
InsecureSkipTLSVerify: false,
CertificateAuthorityData: cluster.CertificateAuthorityData,
ProxyURL: cluster.ProxyURL,
}},
// Define a context that connects the auth info and cluster, and set it as the default
Contexts: map[string]*clientcmdapi.Context{DefaultKubeConfigContext: {
Cluster: kubeConfigCtx.Cluster,
AuthInfo: DefaultKubeConfigAuth,
Namespace: "configuration",
}},
CurrentContext: DefaultKubeConfigContext,
}

return kubeconfig, nil
}

func currentKubeConfigCluster(config *clientcmdapi.Config) (*clientcmdapi.Context, *clientcmdapi.Cluster, error) {
kubeConfigCtx, ok := config.Contexts[config.CurrentContext]
if !ok {
return nil, nil, fmt.Errorf("kubeconfig does not contains context: %s", config.CurrentContext)
}

cluster, ok := config.Clusters[kubeConfigCtx.Cluster]
if !ok {
return nil, nil, fmt.Errorf("kubeconfig does not contains cluster: %s", kubeConfigCtx.Cluster)
}

return kubeConfigCtx, cluster, nil
}

// The hub kubeconfig is valid when it shares the same value of the following with the
// bootstrap hub kubeconfig.
// 1. The hub server
// 2. The proxy url
// 3. The CA bundle
// 4. The current context cluster name
func IsHubKubeconfigValid(bootstrapKubeConfig, hubeKubeConfig *clientcmdapi.Config) (bool, error) {
if bootstrapKubeConfig == nil {
return false, nil
}
bootstrapCtx, bootstrapCluster, err := currentKubeConfigCluster(bootstrapKubeConfig)
if err != nil {
return false, err
}

if hubeKubeConfig == nil {
return false, nil
}
hubKubeConfigCtx, hubKubeConfigCluster, err := currentKubeConfigCluster(hubeKubeConfig)
switch {
case err != nil:
return false, err
case bootstrapCluster.Server != hubKubeConfigCluster.Server,
bootstrapCluster.ProxyURL != hubKubeConfigCluster.ProxyURL,
!reflect.DeepEqual(bootstrapCluster.CertificateAuthorityData, hubKubeConfigCluster.CertificateAuthorityData),
// Here in addition to the server, proxyURL and CA bundle, we also need to compare the cluster name,
// because in some cases even the hub cluster API server serving certificate(kubeconfig ca bundle)
// is the same, but the signer certificate may be different(i.e the hub kubernetes cluster is rebuilt
// with a same serving certificate and url), so setting the cluster name in the bootstrap kubeconfig
// can help to distinguish the different clusters(signer certificate). And comparing the cluster name
// can help to avoid the situation that the hub kubeconfig is valid but not for the current cluster.
bootstrapCtx.Cluster != hubKubeConfigCtx.Cluster:
return false, nil
default:
return true, nil
}
}
105 changes: 105 additions & 0 deletions pkg/registration/register/common_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
package register

import (
"reflect"
"testing"

clientcmdapi "k8s.io/client-go/tools/clientcmd/api"
)

func TestBaseKubeConfigFromBootStrap(t *testing.T) {
server1 := "https://127.0.0.1:6443"
server2 := "https://api.cluster1.example.com:6443"
caData1 := []byte("fake-ca-data1")
caData2 := []byte("fake-ca-data2")
proxyURL := "https://127.0.0.1:3129"

cases := []struct {
name string
kubeconfig clientcmdapi.Config
expectedServer string
expectedCAData []byte
expectedProxyURL string
}{
{
name: "without proxy url",
kubeconfig: clientcmdapi.Config{
Clusters: map[string]*clientcmdapi.Cluster{
"test-cluster": {
Server: server1,
CertificateAuthorityData: caData1,
}},
// Define a context that connects the auth info and cluster, and set it as the default
Contexts: map[string]*clientcmdapi.Context{DefaultKubeConfigContext: {
Cluster: "test-cluster",
AuthInfo: DefaultKubeConfigAuth,
Namespace: "configuration",
}},
CurrentContext: DefaultKubeConfigContext,
AuthInfos: map[string]*clientcmdapi.AuthInfo{
DefaultKubeConfigAuth: {
ClientCertificate: "tls.crt",
ClientKey: "tls.key",
},
},
},
expectedServer: server1,
expectedCAData: caData1,
expectedProxyURL: "",
},
{
name: "with proxy url",
kubeconfig: clientcmdapi.Config{
Clusters: map[string]*clientcmdapi.Cluster{
"test-cluster": {
Server: server2,
CertificateAuthorityData: caData2,
ProxyURL: proxyURL,
}},
// Define a context that connects the auth info and cluster, and set it as the default
Contexts: map[string]*clientcmdapi.Context{DefaultKubeConfigContext: {
Cluster: "test-cluster",
AuthInfo: DefaultKubeConfigAuth,
Namespace: "configuration",
}},
CurrentContext: DefaultKubeConfigContext,
AuthInfos: map[string]*clientcmdapi.AuthInfo{
DefaultKubeConfigAuth: {
ClientCertificate: "tls.crt",
ClientKey: "tls.key",
},
},
},
expectedServer: server2,
expectedCAData: caData2,
expectedProxyURL: proxyURL,
},
}
for _, c := range cases {
t.Run(c.name, func(t *testing.T) {
config, err := BaseKubeConfigFromBootStrap(&c.kubeconfig)
if err != nil {
t.Errorf("unexpected error: %v", err)
}

cluster := config.Contexts[DefaultKubeConfigContext].Cluster

if cluster != "test-cluster" {
t.Errorf("expect context cluster %s, but %s", "test-cluster",
config.Contexts[DefaultKubeConfigContext].Cluster)
}

if c.expectedServer != config.Clusters[cluster].Server {
t.Errorf("expect server %s, but %s", c.expectedServer, config.Clusters[cluster].Server)
}

if c.expectedProxyURL != config.Clusters[cluster].ProxyURL {
t.Errorf("expect proxy url %s, but %s", c.expectedProxyURL, proxyURL)
}

if !reflect.DeepEqual(c.expectedCAData, config.Clusters[cluster].CertificateAuthorityData) {
t.Errorf("expect ca data %v, but %v", c.expectedCAData, config.Clusters[cluster].CertificateAuthorityData)
}
})
}
}
Loading

0 comments on commit 5eed113

Please sign in to comment.