Skip to content

Commit

Permalink
Address review comments
Browse files Browse the repository at this point in the history 
Signed-off-by: Furkat Gofurov <furkat.gofurov@suse.com>
  • Loading branch information
@furkatgofurov7
furkatgofurov7 committed Aug 2, 2024
1 parent 4a26f2f commit 792e3b0
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 26 deletions.
29 changes: 13 additions & 16 deletions exp/etcdrestore/webhooks/rke2config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ func (r *RKE2ConfigWebhook) Default(ctx context.Context, obj runtime.Object) err
return apierrors.NewBadRequest(fmt.Sprintf("failed to create secret plan resources: %s", err))
}

serviceAccountToken, err := r.EnsureServiceAccountSecretPopulated(ctx, planSecretName)
serviceAccountToken, err := r.ensureServiceAccountSecretPopulated(ctx, planSecretName)
if err != nil {
return apierrors.NewBadRequest(fmt.Sprintf("failed to ensure service account secret is populated: %s", err))
}
Expand Down Expand Up @@ -127,15 +127,15 @@ func (r *RKE2ConfigWebhook) Default(ctx context.Context, obj runtime.Object) err
return apierrors.NewBadRequest(fmt.Sprintf("failed to get ca setting: %s", err))
}

if err := r.CreateConnectInfoJson(ctx, rke2Config, planSecretName, serverUrl, pem, serviceAccountToken); err != nil {
if err := r.createConnectInfoJson(ctx, rke2Config, planSecretName, serverUrl, pem, serviceAccountToken); err != nil {
return apierrors.NewBadRequest(fmt.Sprintf("failed to create connect info json: %s", err))
}

if err := r.CreateSystemAgentInstallScript(ctx, serverUrl, rke2Config); err != nil {
if err := r.createSystemAgentInstallScript(ctx, serverUrl, rke2Config); err != nil {
return apierrors.NewBadRequest(fmt.Sprintf("failed to create system agent install script: %s", err))
}

if err := r.CreateConfigYAML(rke2Config); err != nil {
if err := r.createConfigYAML(rke2Config); err != nil {
return apierrors.NewBadRequest(fmt.Sprintf("failed to create config.yaml: %s", err))
}

Expand All @@ -152,14 +152,11 @@ func (r *RKE2ConfigWebhook) createSecretPlanResources(ctx context.Context, planS

var errs []error

// Create the ServiceAccount first to later pass to the RoleBinding creation
sa := r.createServiceAccount(planSecretName, rke2Config)

resources := []client.Object{
sa,
r.createServiceAccount(planSecretName, rke2Config),
r.createSecret(planSecretName, rke2Config),
r.createRole(planSecretName, rke2Config),
r.createRoleBinding(sa.Name, sa.Namespace, planSecretName, rke2Config),
r.createRoleBinding(planSecretName, rke2Config),
r.createServiceAccountSecret(planSecretName, rke2Config),
}

Expand Down Expand Up @@ -225,7 +222,7 @@ func (r *RKE2ConfigWebhook) createRole(planSecretName string, rke2Config *bootst
}

// createRoleBinding creates a RoleBinding for the plan.
func (r *RKE2ConfigWebhook) createRoleBinding(serviceAccountName, serviceAccountNamespace, planSecretName string, rke2Config *bootstrapv1.RKE2Config) *rbacv1.RoleBinding {
func (r *RKE2ConfigWebhook) createRoleBinding(planSecretName string, rke2Config *bootstrapv1.RKE2Config) *rbacv1.RoleBinding {
return &rbacv1.RoleBinding{
ObjectMeta: metav1.ObjectMeta{
Name: planSecretName,
Expand All @@ -234,8 +231,8 @@ func (r *RKE2ConfigWebhook) createRoleBinding(serviceAccountName, serviceAccount
Subjects: []rbacv1.Subject{
{
Kind: "ServiceAccount",
Name: serviceAccountName,
Namespace: serviceAccountNamespace,
Name: planSecretName,
Namespace: rke2Config.Namespace,
},
},
RoleRef: rbacv1.RoleRef{
Expand Down Expand Up @@ -264,7 +261,7 @@ func (r *RKE2ConfigWebhook) createServiceAccountSecret(planSecretName string, rk
}

// ensureServiceAccountSecretPopulated ensures the ServiceAccount secret is populated.
func (r *RKE2ConfigWebhook) EnsureServiceAccountSecretPopulated(ctx context.Context, planSecretName string) ([]byte, error) {
func (r *RKE2ConfigWebhook) ensureServiceAccountSecretPopulated(ctx context.Context, planSecretName string) ([]byte, error) {
logger := log.FromContext(ctx)

logger.Info("Ensuring service account secret is populated")
Expand Down Expand Up @@ -306,7 +303,7 @@ func (r *RKE2ConfigWebhook) EnsureServiceAccountSecretPopulated(ctx context.Cont
}

// createConnectInfoJson creates the connect-info-config.json file.
func (r *RKE2ConfigWebhook) CreateConnectInfoJson(ctx context.Context, rke2Config *bootstrapv1.RKE2Config, planSecretName, serverUrl, pem string, serviceAccountToken []byte) error {
func (r *RKE2ConfigWebhook) createConnectInfoJson(ctx context.Context, rke2Config *bootstrapv1.RKE2Config, planSecretName, serverUrl, pem string, serviceAccountToken []byte) error {
connectInfoJsonPath := "/etc/rancher/agent/connect-info-config.json"

filePaths := make(map[string]struct{})
Expand Down Expand Up @@ -400,7 +397,7 @@ func (r *RKE2ConfigWebhook) CreateConnectInfoJson(ctx context.Context, rke2Confi
}

// createSystemAgentInstallScript creates the system-agent-install.sh script.
func (r *RKE2ConfigWebhook) CreateSystemAgentInstallScript(ctx context.Context, serverUrl string, rke2Config *bootstrapv1.RKE2Config) error {
func (r *RKE2ConfigWebhook) createSystemAgentInstallScript(ctx context.Context, serverUrl string, rke2Config *bootstrapv1.RKE2Config) error {
systemAgentInstallScriptPath := "/opt/system-agent-install.sh"

filePaths := make(map[string]struct{})
Expand Down Expand Up @@ -456,7 +453,7 @@ func (r *RKE2ConfigWebhook) CreateSystemAgentInstallScript(ctx context.Context,
}

// createConfigYAML creates the config.yaml file.
func (r *RKE2ConfigWebhook) CreateConfigYAML(rke2Config *bootstrapv1.RKE2Config) error {
func (r *RKE2ConfigWebhook) createConfigYAML(rke2Config *bootstrapv1.RKE2Config) error {
configYAMLPath := "/etc/rancher/agent/config.yaml"

filePaths := make(map[string]struct{})
Expand Down
20 changes: 10 additions & 10 deletions exp/etcdrestore/webhooks/rke2config_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -147,13 +147,13 @@ var _ = Describe("RKE2ConfigWebhook tests", func() {
})

It("Should create a role binding with the correct properties", func() {
roleBinding := r.createRoleBinding(serviceAccountName, serviceAccountNamespace, planSecretName, rke2Config)
roleBinding := r.createRoleBinding(planSecretName, rke2Config)

Expect(roleBinding.ObjectMeta.Name).To(Equal(planSecretName))
Expect(roleBinding.ObjectMeta.Namespace).To(Equal(rke2Config.Namespace))
Expect(roleBinding.Subjects[0].Kind).To(Equal("ServiceAccount"))
Expect(roleBinding.Subjects[0].Name).To(Equal(serviceAccountName))
Expect(roleBinding.Subjects[0].Namespace).To(Equal(serviceAccountNamespace))
Expect(roleBinding.Subjects[0].Name).To(Equal(planSecretName))
Expect(roleBinding.Subjects[0].Namespace).To(Equal(rke2Config.Namespace))
Expect(roleBinding.RoleRef.APIGroup).To(Equal(rbacv1.GroupName))
Expect(roleBinding.RoleRef.Kind).To(Equal("Role"))
Expect(roleBinding.RoleRef.Name).To(Equal(planSecretName))
Expand All @@ -170,13 +170,13 @@ var _ = Describe("RKE2ConfigWebhook tests", func() {
})

It("Should return service account token when secret is present and populated", func() {
token, err := r.EnsureServiceAccountSecretPopulated(ctx, planSecretName)
token, err := r.ensureServiceAccountSecretPopulated(ctx, planSecretName)
Expect(err).ToNot(HaveOccurred())
Expect(token).To(Equal([]byte("test-token")))
})

It("Should add connect-info-config.json when it's not present", func() {
err := r.CreateConnectInfoJson(ctx, rke2Config, "plan-secret", serverUrl, pem, token)
err := r.createConnectInfoJson(ctx, rke2Config, "plan-secret", serverUrl, pem, token)
Expect(err).ToNot(HaveOccurred())

Expect(rke2Config.Spec.Files).To(ContainElement(bootstrapv1.File{
Expand All @@ -197,14 +197,14 @@ var _ = Describe("RKE2ConfigWebhook tests", func() {
Path: "/etc/rancher/agent/connect-info-config.json",
})

err := r.CreateConnectInfoJson(ctx, rke2Config, "plan-secret", serverUrl, pem, token)
err := r.createConnectInfoJson(ctx, rke2Config, "plan-secret", serverUrl, pem, token)
Expect(err).ToNot(HaveOccurred())

Expect(rke2Config.Spec.Files).To(HaveLen(1))
})

It("Should add system-agent-install.sh when it's not present", func() {
err := r.CreateSystemAgentInstallScript(ctx, serverUrl, rke2Config)
err := r.createSystemAgentInstallScript(ctx, serverUrl, rke2Config)
Expect(err).ToNot(HaveOccurred())

Expect(rke2Config.Spec.Files).To(ContainElement(bootstrapv1.File{
Expand All @@ -225,14 +225,14 @@ var _ = Describe("RKE2ConfigWebhook tests", func() {
Path: "/opt/system-agent-install.sh",
})

err := r.CreateSystemAgentInstallScript(ctx, serverUrl, rke2Config)
err := r.createSystemAgentInstallScript(ctx, serverUrl, rke2Config)
Expect(err).ToNot(HaveOccurred())

Expect(rke2Config.Spec.Files).To(HaveLen(1))
})

It("Should add config.yaml when it's not present", func() {
err := r.CreateConfigYAML(rke2Config)
err := r.createConfigYAML(rke2Config)
Expect(err).ToNot(HaveOccurred())

Expect(rke2Config.Spec.Files).To(ContainElement(bootstrapv1.File{
Expand All @@ -253,7 +253,7 @@ preserveWorkDirectory: true`,
Path: "/etc/rancher/agent/config.yaml",
})

err := r.CreateConfigYAML(rke2Config)
err := r.createConfigYAML(rke2Config)
Expect(err).ToNot(HaveOccurred())

Expect(rke2Config.Spec.Files).To(HaveLen(1))
Expand Down

0 comments on commit 792e3b0

Please sign in to comment.