Skip to content

Commit

Permalink
[bug-1120]: Update Authorization sidecar to use insecure flag (#442)
Browse files Browse the repository at this point in the history 
* fix auth cert

* remove constant

* remove duplicate verbs
  • Loading branch information
@atye
atye authored Jan 29, 2024
1 parent 2839e5c commit d76a67d
Show file tree
Hide file tree
Showing 4 changed files with 108 additions and 31 deletions.
24 changes: 21 additions & 3 deletions pkg/modules/authorization.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ NAME_LOOP:
}

// CheckApplyContainersAuth --
func CheckApplyContainersAuth(containers []acorev1.ContainerApplyConfiguration, drivertype string) error {
func CheckApplyContainersAuth(containers []acorev1.ContainerApplyConfiguration, drivertype string, skipCertificateValidation bool) error {
authString := "karavi-authorization-proxy"
for _, cnt := range containers {
if *cnt.Name == authString {
Expand All @@ -189,10 +189,20 @@ func CheckApplyContainersAuth(containers []acorev1.ContainerApplyConfiguration,
}

for _, env := range cnt.Env {
if *env.Name == "SKIP_CERTIFICATE_VALIDATION" {
if *env.Name == "SKIP_CERTIFICATE_VALIDATION" || *env.Name == "INSECURE" {
if _, err := strconv.ParseBool(*env.Value); err != nil {
return fmt.Errorf("%s is an invalid value for SKIP_CERTIFICATE_VALIDATION: %v", *env.Value, err)
}

if skipCertificateValidation {
if *env.Value != "true" {
return fmt.Errorf("expected SKIP_CERTIFICATE_VALIDATION/INSECURE to be true")
}
} else {
if *env.Value != "false" {
return fmt.Errorf("expected SKIP_CERTIFICATE_VALIDATION/INSECURE to be false")
}
}
}
if *env.Name == "PROXY_HOST" && *env.Value == "" {
return fmt.Errorf("PROXY_HOST for authorization is empty")
Expand Down Expand Up @@ -243,7 +253,7 @@ func getAuthApplyCR(cr csmv1.ContainerStorageModule, op utils.OperatorConfig) (*

skipCertValid := false
for _, env := range authModule.Components[0].Envs {
if env.Name == "SKIP_CERTIFICATE_VALIDATION" {
if env.Name == "INSECURE" || env.Name == "SKIP_CERTIFICATE_VALIDATION" {
skipCertValid, _ = strconv.ParseBool(env.Value)
}
}
Expand All @@ -256,6 +266,14 @@ func getAuthApplyCR(cr csmv1.ContainerStorageModule, op utils.OperatorConfig) (*
container.VolumeMounts = container.VolumeMounts[:len(container.VolumeMounts)-1]
}

}
} else {
for i, e := range container.Env {
if *e.Name == "INSECURE" || *e.Name == "SKIP_CERTIFICATE_VALIDATION" {
value := strconv.FormatBool(skipCertValid)
container.Env[i].Value = &value
}

}
}

Expand Down
81 changes: 57 additions & 24 deletions pkg/modules/authorization_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ func TestCheckApplyContainersAuth(t *testing.T) {
*acorev1.Container().WithName("karavi-authorization-proxy"),
}
driver := "powerscale"
err := CheckApplyContainersAuth(got, driver)
err := CheckApplyContainersAuth(got, driver, true)
if err == nil {
t.Errorf("got %v, expected karavi-authorization-config to be injected", got)
}
Expand All @@ -83,7 +83,7 @@ func TestCheckApplyContainersAuth(t *testing.T) {
t.Run("it handles an empty container", func(t *testing.T) {
got := []acorev1.ContainerApplyConfiguration{}
driver := "powerscale"
err := CheckApplyContainersAuth(got, driver)
err := CheckApplyContainersAuth(got, driver, true)
if err == nil {
t.Errorf("got %v, expected karavi-authorization-config to be injected", got)
}
Expand All @@ -92,7 +92,7 @@ func TestCheckApplyContainersAuth(t *testing.T) {

func TestAuthInjectDaemonset(t *testing.T) {
ctx := context.Background()
correctlyInjected := func(ds applyv1.DaemonSetApplyConfiguration, drivertype string) error {
correctlyInjected := func(ds applyv1.DaemonSetApplyConfiguration, drivertype string, skipCertificateValidation bool) error {
err := CheckAnnotationAuth(ds.Annotations)
if err != nil {
return err
Expand All @@ -102,15 +102,15 @@ func TestAuthInjectDaemonset(t *testing.T) {
return err
}

err = CheckApplyContainersAuth(ds.Spec.Template.Spec.Containers, drivertype)
err = CheckApplyContainersAuth(ds.Spec.Template.Spec.Containers, drivertype, skipCertificateValidation)
if err != nil {
return err
}
return nil
}
//*appsv1.DaemonSet
tests := map[string]func(t *testing.T) (bool, applyv1.DaemonSetApplyConfiguration, utils.OperatorConfig){
"success - greenfield injection": func(*testing.T) (bool, applyv1.DaemonSetApplyConfiguration, utils.OperatorConfig) {
tests := map[string]func(t *testing.T) (bool, bool, applyv1.DaemonSetApplyConfiguration, utils.OperatorConfig){
"success - greenfield injection": func(*testing.T) (bool, bool, applyv1.DaemonSetApplyConfiguration, utils.OperatorConfig) {
customResource, err := getCustomResource("./testdata/cr_powerscale_auth.yaml")
if err != nil {
panic(err)
Expand All @@ -120,9 +120,9 @@ func TestAuthInjectDaemonset(t *testing.T) {
panic(err)
}

return true, nodeYAML.DaemonSetApplyConfig, operatorConfig
return true, true, nodeYAML.DaemonSetApplyConfig, operatorConfig
},
"success - brownfield injection": func(*testing.T) (bool, applyv1.DaemonSetApplyConfiguration, utils.OperatorConfig) {
"success - brownfield injection": func(*testing.T) (bool, bool, applyv1.DaemonSetApplyConfiguration, utils.OperatorConfig) {
customResource, err := getCustomResource("./testdata/cr_powerscale_auth.yaml")
if err != nil {
panic(err)
Expand All @@ -136,9 +136,25 @@ func TestAuthInjectDaemonset(t *testing.T) {
panic(err)
}

return true, *newDaemonSet, operatorConfig
return true, true, *newDaemonSet, operatorConfig
},
"fail - bad config path": func(*testing.T) (bool, applyv1.DaemonSetApplyConfiguration, utils.OperatorConfig) {
"success - validate certificate": func(*testing.T) (bool, bool, applyv1.DaemonSetApplyConfiguration, utils.OperatorConfig) {
customResource, err := getCustomResource("./testdata/cr_powerscale_auth_validate_cert.yaml")
if err != nil {
panic(err)
}
nodeYAML, err := drivers.GetNode(ctx, customResource, operatorConfig, csmv1.PowerScaleName, "node.yaml")
if err != nil {
panic(err)
}
newDaemonSet, err := AuthInjectDaemonset(nodeYAML.DaemonSetApplyConfig, customResource, operatorConfig)
if err != nil {
panic(err)
}

return true, false, *newDaemonSet, operatorConfig
},
"fail - bad config path": func(*testing.T) (bool, bool, applyv1.DaemonSetApplyConfiguration, utils.OperatorConfig) {
customResource, err := getCustomResource("./testdata/cr_powerscale_auth.yaml")
if err != nil {
panic(err)
Expand All @@ -150,20 +166,20 @@ func TestAuthInjectDaemonset(t *testing.T) {
tmpOperatorConfig := operatorConfig
tmpOperatorConfig.ConfigDirectory = "bad/path"

return false, nodeYAML.DaemonSetApplyConfig, tmpOperatorConfig
return false, false, nodeYAML.DaemonSetApplyConfig, tmpOperatorConfig
},
}
for name, tc := range tests {
t.Run(name, func(t *testing.T) {
success, ds, opConfig := tc(t)
success, skipCertificateValidation, ds, opConfig := tc(t)
customResource, err := getCustomResource("./testdata/cr_powerscale_auth.yaml")
if err != nil {
panic(err)
}
newDaemonSet, err := AuthInjectDaemonset(ds, customResource, opConfig)
if success {
assert.NoError(t, err)
if err := correctlyInjected(*newDaemonSet, string(customResource.Spec.Driver.CSIDriverType)); err != nil {
if err := correctlyInjected(*newDaemonSet, string(customResource.Spec.Driver.CSIDriverType), skipCertificateValidation); err != nil {
assert.NoError(t, err)
}
} else {
Expand All @@ -176,7 +192,7 @@ func TestAuthInjectDaemonset(t *testing.T) {

func TestAuthInjectDeployment(t *testing.T) {
ctx := context.Background()
correctlyInjected := func(dp applyv1.DeploymentApplyConfiguration, drivertype string) error {
correctlyInjected := func(dp applyv1.DeploymentApplyConfiguration, drivertype string, skipCertificateValidation bool) error {
err := CheckAnnotationAuth(dp.Annotations)
if err != nil {
return err
Expand All @@ -185,15 +201,15 @@ func TestAuthInjectDeployment(t *testing.T) {
if err != nil {
return err
}
err = CheckApplyContainersAuth(dp.Spec.Template.Spec.Containers, drivertype)
err = CheckApplyContainersAuth(dp.Spec.Template.Spec.Containers, drivertype, skipCertificateValidation)
if err != nil {
return err
}
return nil
}

tests := map[string]func(t *testing.T) (bool, applyv1.DeploymentApplyConfiguration, utils.OperatorConfig, csmv1.ContainerStorageModule){
"success - greenfield injection": func(*testing.T) (bool, applyv1.DeploymentApplyConfiguration, utils.OperatorConfig, csmv1.ContainerStorageModule) {
tests := map[string]func(t *testing.T) (bool, bool, applyv1.DeploymentApplyConfiguration, utils.OperatorConfig, csmv1.ContainerStorageModule){
"success - greenfield injection": func(*testing.T) (bool, bool, applyv1.DeploymentApplyConfiguration, utils.OperatorConfig, csmv1.ContainerStorageModule) {
customResource, err := getCustomResource("./testdata/cr_powerscale_auth.yaml")
if err != nil {
panic(err)
Expand All @@ -202,9 +218,9 @@ func TestAuthInjectDeployment(t *testing.T) {
if err != nil {
panic(err)
}
return true, controllerYAML.Deployment, operatorConfig, customResource
return true, true, controllerYAML.Deployment, operatorConfig, customResource
},
"success - brownfield injection": func(*testing.T) (bool, applyv1.DeploymentApplyConfiguration, utils.OperatorConfig, csmv1.ContainerStorageModule) {
"success - brownfield injection": func(*testing.T) (bool, bool, applyv1.DeploymentApplyConfiguration, utils.OperatorConfig, csmv1.ContainerStorageModule) {
customResource, err := getCustomResource("./testdata/cr_powerscale_auth.yaml")
if err != nil {
panic(err)
Expand All @@ -219,9 +235,26 @@ func TestAuthInjectDeployment(t *testing.T) {
panic(err)
}

return true, *newDeployment, operatorConfig, customResource
return true, true, *newDeployment, operatorConfig, customResource
},
"success - validate certificate": func(*testing.T) (bool, bool, applyv1.DeploymentApplyConfiguration, utils.OperatorConfig, csmv1.ContainerStorageModule) {
customResource, err := getCustomResource("./testdata/cr_powerscale_auth_validate_cert.yaml")
if err != nil {
panic(err)
}
tmpCR := customResource
controllerYAML, err := drivers.GetController(ctx, tmpCR, operatorConfig, csmv1.PowerScaleName)
if err != nil {
panic(err)
}
newDeployment, err := AuthInjectDeployment(controllerYAML.Deployment, tmpCR, operatorConfig)
if err != nil {
panic(err)
}

return true, false, *newDeployment, operatorConfig, customResource
},
"fail - bad config path": func(*testing.T) (bool, applyv1.DeploymentApplyConfiguration, utils.OperatorConfig, csmv1.ContainerStorageModule) {
"fail - bad config path": func(*testing.T) (bool, bool, applyv1.DeploymentApplyConfiguration, utils.OperatorConfig, csmv1.ContainerStorageModule) {
customResource, err := getCustomResource("./testdata/cr_powerscale_auth.yaml")
if err != nil {
panic(err)
Expand All @@ -233,16 +266,16 @@ func TestAuthInjectDeployment(t *testing.T) {
tmpOperatorConfig := operatorConfig
tmpOperatorConfig.ConfigDirectory = "bad/path"

return false, controllerYAML.Deployment, tmpOperatorConfig, customResource
return false, true, controllerYAML.Deployment, tmpOperatorConfig, customResource
},
}
for name, tc := range tests {
t.Run(name, func(t *testing.T) {
success, dp, opConfig, cr := tc(t)
success, skipCertificateValidation, dp, opConfig, cr := tc(t)
newDeployment, err := AuthInjectDeployment(dp, cr, opConfig)
if success {
assert.NoError(t, err)
if err := correctlyInjected(*newDeployment, string(cr.Spec.Driver.CSIDriverType)); err != nil {
if err := correctlyInjected(*newDeployment, string(cr.Spec.Driver.CSIDriverType), skipCertificateValidation); err != nil {
assert.NoError(t, err)
}
} else {
Expand Down
30 changes: 30 additions & 0 deletions pkg/modules/testdata/cr_powerscale_auth_validate_cert.yaml
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
apiVersion: storage.dell.com/v1
kind: ContainerStorageModule
metadata:
name: isilon
namespace: isilon
spec:
driver:
csiDriverType: "isilon"
configVersion: v2.9.0
authSecret: isilon-creds-custom
replicas: 1
common:
image: "dellemc/csi-isilon:v2.9.0"
imagePullPolicy: IfNotPresent

modules:
- name: authorization
# enable: Enable/Disable csm-authorization
enabled: true
components:
- name: karavi-authorization-proxy
image: dellemc/csm-authorization-sidecar:v1.7.0
envs:
# proxyHost: hostname of the csm-authorization server
- name: "PROXY_HOST"
value: "testing-proxy-host"

# skipCertificateValidation: Enable/Disable certificate validation of the csm-authorization server
- name: "SKIP_CERTIFICATE_VALIDATION"
value: "false"
4 changes: 0 additions & 4 deletions tests/config/driverconfig/powerflex/v2.9.1/controller.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ rules:
- apiGroups: [""]
resources: ["nodes"]
verbs: ["get", "list", "watch", "patch"]
verbs: ["get", "list", "watch"]
- apiGroups: [""]
resources: ["persistentvolumes"]
verbs: ["get", "list", "watch", "create", "delete", "update", "patch"]
Expand All @@ -34,7 +33,6 @@ rules:
- apiGroups: ["storage.k8s.io"]
resources: ["volumeattachments"]
verbs: ["get", "list", "watch", "update", "patch", "delete"]
verbs: ["get", "list", "watch", "update", "patch"]
- apiGroups: ["storage.k8s.io"]
resources: ["csinodes"]
verbs: ["get", "list", "watch", "update"]
Expand All @@ -47,7 +45,6 @@ rules:
- apiGroups: [""]
resources: ["pods"]
verbs: ["get", "list", "watch", "update", "delete"]
verbs: ["get", "list", "watch"]
# below for snapshotter
- apiGroups: [""]
resources: ["secrets"]
Expand All @@ -61,7 +58,6 @@ rules:
- apiGroups: ["snapshot.storage.k8s.io"]
resources: ["volumesnapshots"]
verbs: ["get", "list", "watch", "update", "create", "delete"]
verbs: ["get", "list", "watch", "update"]
- apiGroups: ["snapshot.storage.k8s.io"]
resources: ["volumesnapshots/status","volumesnapshotcontents/status"]
verbs: ["get", "list", "watch", "update", "patch"]
Expand Down

0 comments on commit d76a67d

Please sign in to comment.