Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix BackendConfig.securityPolicy is not removed after config updates #1749

Merged
merged 2 commits into from
Aug 10, 2022
Merged

Fix BackendConfig.securityPolicy is not removed after config updates #1749

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0a976eb
Fix BackendConfig.securityPolicy is not removed after config updates
ruixiansong Jul 19, 2022
0289885
Update securityPolicy test cases
ruixiansong Jul 27, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 10 additions & 7 deletions pkg/backends/features/securitypolicy.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,24 +31,27 @@ import (
// EnsureSecurityPolicy ensures the security policy link on backend service.
// TODO(mrhohn): Emit event when attach/detach security policy to backend service.
func EnsureSecurityPolicy(cloud *gce.Cloud, sp utils.ServicePort, be *composite.BackendService) error {
if sp.BackendConfig.Spec.SecurityPolicy == nil {
return nil
}

if be.Scope != meta.Global {
return fmt.Errorf("cloud armor security policies not supported for %s backend service %s", be.Scope, be.Name)
var desiredPolicyName string
if sp.BackendConfig.Spec.SecurityPolicy != nil {
desiredPolicyName = sp.BackendConfig.Spec.SecurityPolicy.Name
} else {
desiredPolicyName = ""
}

existingPolicyName, err := utils.KeyName(be.SecurityPolicy)
// The parser returns error for empty values.
if be.SecurityPolicy != "" && err != nil {
return err
}
desiredPolicyName := sp.BackendConfig.Spec.SecurityPolicy.Name

if existingPolicyName == desiredPolicyName {
return nil
}

if be.Scope != meta.Global {
return fmt.Errorf("cloud armor security policies not supported for %s backend service %s", be.Scope, be.Name)
}

klog.V(2).Infof("Set security policy in backend service %s (%s:%s) to %q", be.Name, sp.ID.Service.String(), sp.ID.Port.String(), desiredPolicyName)
if err := composite.SetSecurityPolicy(cloud, be, desiredPolicyName); err != nil {
return fmt.Errorf("failed to set security policy %q for backend service %s (%s:%s): %v", desiredPolicyName, be.Name, sp.ID.Service.String(), sp.ID.Port.String(), err)
Expand Down
27 changes: 17 additions & 10 deletions pkg/backends/features/securitypolicy_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ func TestEnsureSecurityPolicy(t *testing.T) {
expectSetCall: true,
},
{
desc: "remove-policy",
desc: "unset-policy-empty-policy-string",
currentBackendService: &composite.BackendService{
Scope: meta.Global,
SecurityPolicy: "https://www.googleapis.com/compute/projects/test-project/global/securityPolicies/policy-1",
Expand All @@ -92,6 +92,15 @@ func TestEnsureSecurityPolicy(t *testing.T) {
},
expectSetCall: true,
},
{
desc: "unset-policy-no-specified-policy",
currentBackendService: &composite.BackendService{
Scope: meta.Global,
SecurityPolicy: "https://www.googleapis.com/compute/projects/test-project/global/securityPolicies/policy-1",
},
desiredConfig: &backendconfigv1.BackendConfig{},
expectSetCall: true,
},
{
desc: "same-policy",
currentBackendService: &composite.BackendService{
Expand All @@ -111,15 +120,6 @@ func TestEnsureSecurityPolicy(t *testing.T) {
currentBackendService: &composite.BackendService{Scope: meta.Global},
desiredConfig: &backendconfigv1.BackendConfig{},
},
{
desc: "no-specified-policy",
currentBackendService: &composite.BackendService{
Scope: meta.Global,
SecurityPolicy: "https://www.googleapis.com/compute/projects/test-project/global/securityPolicies/policy-1",
},
desiredConfig: &backendconfigv1.BackendConfig{},
expectSetCall: false,
},
{
desc: "regional backend service",
currentBackendService: &composite.BackendService{
Expand All @@ -135,6 +135,13 @@ func TestEnsureSecurityPolicy(t *testing.T) {
expectSetCall: false,
expectError: true,
},
{
desc: "regional backend service with no specified security policy",
currentBackendService: &composite.BackendService{Scope: meta.Regional},
desiredConfig: &backendconfigv1.BackendConfig{},
expectSetCall: false,
expectError: false,
},
}

for i, tc := range testCases {
Expand Down