Skip to content

Commit

Permalink
move nsg related code to securitygroup package
Browse files Browse the repository at this point in the history 
Signed-off-by: Fan Shang Xiang <[email protected]>
  • Loading branch information
@MartinForReal
MartinForReal authored and k8s-infra-cherrypick-robot committed Oct 17, 2024
1 parent 728a91a commit d0b7bcb
Show file tree
Hide file tree
Showing 28 changed files with 234 additions and 108 deletions.
6 changes: 3 additions & 3 deletions internal/testutil/fixture/azure_securitygroup.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,9 @@ import (

"sigs.k8s.io/cloud-provider-azure/internal/testutil"
"sigs.k8s.io/cloud-provider-azure/pkg/consts"
"sigs.k8s.io/cloud-provider-azure/pkg/provider/loadbalancer/fnutil"
"sigs.k8s.io/cloud-provider-azure/pkg/provider/loadbalancer/iputil"
"sigs.k8s.io/cloud-provider-azure/pkg/provider/loadbalancer/securitygroup"
"sigs.k8s.io/cloud-provider-azure/pkg/provider/securitygroup"
fnutil "sigs.k8s.io/cloud-provider-azure/pkg/util/collectionutil"
"sigs.k8s.io/cloud-provider-azure/pkg/util/iputil"
)

// NoiseSecurityRules returns 3 non cloud-provider-specific security rules.
Expand Down
2 changes: 1 addition & 1 deletion internal/testutil/fixture/fixture.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import (
"math/big"
"net/netip"

"sigs.k8s.io/cloud-provider-azure/pkg/provider/loadbalancer/fnutil"
fnutil "sigs.k8s.io/cloud-provider-azure/pkg/util/collectionutil"
)

type Fixture struct{}
Expand Down
24 changes: 14 additions & 10 deletions pkg/provider/azure.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ import (
azcache "sigs.k8s.io/cloud-provider-azure/pkg/cache"
"sigs.k8s.io/cloud-provider-azure/pkg/consts"
ratelimitconfig "sigs.k8s.io/cloud-provider-azure/pkg/provider/config"
"sigs.k8s.io/cloud-provider-azure/pkg/provider/securitygroup"
"sigs.k8s.io/cloud-provider-azure/pkg/retry"
utilsets "sigs.k8s.io/cloud-provider-azure/pkg/util/sets"
"sigs.k8s.io/cloud-provider-azure/pkg/util/taints"
Expand Down Expand Up @@ -423,10 +424,10 @@ type Cloud struct {
routeUpdater batchProcessor
backendPoolUpdater batchProcessor

vmCache azcache.Resource
lbCache azcache.Resource
nsgCache azcache.Resource
rtCache azcache.Resource
vmCache azcache.Resource
lbCache azcache.Resource
nsgRepo securitygroup.Repository
rtCache azcache.Resource
// public ip cache
// key: [resourceGroupName]
// Value: sync.Map of [pipName]*PublicIPAddress
Expand Down Expand Up @@ -727,8 +728,16 @@ func (az *Cloud) InitializeCloudFromConfig(ctx context.Context, config *Config,
if err != nil {
return err
}
}

networkClientFactory := az.NetworkClientFactory
if networkClientFactory == nil {
networkClientFactory = az.ComputeClientFactory
}
az.nsgRepo, err = securitygroup.NewSecurityGroupRepo(az.SecurityGroupResourceGroup, az.SecurityGroupName, az.NsgCacheTTLInSeconds, az.DisableAPICallCache, networkClientFactory.GetSecurityGroupClient())
if err != nil {
return err
}
}
err = az.initCaches()
if err != nil {
return err
Expand Down Expand Up @@ -841,11 +850,6 @@ func (az *Cloud) initCaches() (err error) {
return err
}

az.nsgCache, err = az.newNSGCache()
if err != nil {
return err
}

az.rtCache, err = az.newRouteTableCache()
if err != nil {
return err
Expand Down
3 changes: 2 additions & 1 deletion pkg/provider/azure_fakes.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ import (
azcache "sigs.k8s.io/cloud-provider-azure/pkg/cache"
"sigs.k8s.io/cloud-provider-azure/pkg/consts"
"sigs.k8s.io/cloud-provider-azure/pkg/provider/config"
"sigs.k8s.io/cloud-provider-azure/pkg/provider/securitygroup"
utilsets "sigs.k8s.io/cloud-provider-azure/pkg/util/sets"
)

Expand Down Expand Up @@ -138,7 +139,7 @@ func GetTestCloud(ctrl *gomock.Controller) (az *Cloud) {
az.VMSet, _ = newAvailabilitySet(az)
az.vmCache, _ = az.newVMCache()
az.lbCache, _ = az.newLBCache()
az.nsgCache, _ = az.newNSGCache()
az.nsgRepo, _ = securitygroup.NewSecurityGroupRepo(az.SecurityGroupResourceGroup, az.SecurityGroupName, az.NsgCacheTTLInSeconds, az.Config.DisableAPICallCache, securtyGrouptrack2Client)
az.rtCache, _ = az.newRouteTableCache()
az.pipCache, _ = az.newPIPCache()
az.plsCache, _ = az.newPLSCache()
Expand Down
7 changes: 3 additions & 4 deletions pkg/provider/azure_loadbalancer.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,10 @@ import (
"sigs.k8s.io/cloud-provider-azure/pkg/log"
"sigs.k8s.io/cloud-provider-azure/pkg/metrics"
"sigs.k8s.io/cloud-provider-azure/pkg/provider/loadbalancer"
"sigs.k8s.io/cloud-provider-azure/pkg/provider/loadbalancer/iputil"
"sigs.k8s.io/cloud-provider-azure/pkg/retry"
"sigs.k8s.io/cloud-provider-azure/pkg/trace"
"sigs.k8s.io/cloud-provider-azure/pkg/trace/attributes"
"sigs.k8s.io/cloud-provider-azure/pkg/util/iputil"
utilsets "sigs.k8s.io/cloud-provider-azure/pkg/util/sets"
)

Expand Down Expand Up @@ -2924,7 +2924,7 @@ func (az *Cloud) reconcileSecurityGroup(

var accessControl *loadbalancer.AccessControl
{
sg, err := az.getSecurityGroup(ctx, azcache.CacheReadTypeDefault)
sg, err := az.nsgRepo.GetSecurityGroup(ctx)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -3017,13 +3017,12 @@ func (az *Cloud) reconcileSecurityGroup(
if updated {
logger.V(2).Info("Preparing to update security group")
logger.V(5).Info("CreateOrUpdateSecurityGroup begin")
err := az.CreateOrUpdateSecurityGroup(rv)
err := az.nsgRepo.CreateOrUpdateSecurityGroup(ctx, rv)
if err != nil {
logger.Error(err, "Failed to update security group")
return nil, err
}
logger.V(5).Info("CreateOrUpdateSecurityGroup end")
_ = az.nsgCache.Delete(ptr.Deref(rv.Name, ""))
}
return rv, nil
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/provider/azure_loadbalancer_accesscontrol.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import (
"sigs.k8s.io/cloud-provider-azure/pkg/consts"
"sigs.k8s.io/cloud-provider-azure/pkg/log"
"sigs.k8s.io/cloud-provider-azure/pkg/provider/loadbalancer"
"sigs.k8s.io/cloud-provider-azure/pkg/provider/loadbalancer/fnutil"
fnutil "sigs.k8s.io/cloud-provider-azure/pkg/util/collectionutil"
)

func filterServicesByIngressIPs(services []*v1.Service, ips []netip.Addr) []*v1.Service {
Expand Down
4 changes: 2 additions & 2 deletions pkg/provider/azure_loadbalancer_accesscontrol_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,9 @@ import (
"sigs.k8s.io/cloud-provider-azure/pkg/consts"
"sigs.k8s.io/cloud-provider-azure/pkg/log"
"sigs.k8s.io/cloud-provider-azure/pkg/provider/loadbalancer"
"sigs.k8s.io/cloud-provider-azure/pkg/provider/loadbalancer/iputil"
"sigs.k8s.io/cloud-provider-azure/pkg/provider/loadbalancer/securitygroup"
"sigs.k8s.io/cloud-provider-azure/pkg/provider/securitygroup"
"sigs.k8s.io/cloud-provider-azure/pkg/retry"
"sigs.k8s.io/cloud-provider-azure/pkg/util/iputil"
)

func TestCloud_reconcileSecurityGroup(t *testing.T) {
Expand Down
18 changes: 0 additions & 18 deletions pkg/provider/azure_wrap.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,22 +17,18 @@ limitations under the License.
package provider

import (
"errors"
"fmt"
"net/http"
"regexp"
"strings"

"github.com/Azure/azure-sdk-for-go/sdk/azcore"

"sigs.k8s.io/cloud-provider-azure/pkg/consts"
"sigs.k8s.io/cloud-provider-azure/pkg/retry"
)

var (
vmCacheTTLDefaultInSeconds = 60
loadBalancerCacheTTLDefaultInSeconds = 120
nsgCacheTTLDefaultInSeconds = 120
routeTableCacheTTLDefaultInSeconds = 120
publicIPCacheTTLDefaultInSeconds = 120
plsCacheTTLDefaultInSeconds = 120
Expand All @@ -56,20 +52,6 @@ func checkResourceExistsFromError(err *retry.Error) (bool, *retry.Error) {
return false, err
}

func checkResourceExistsFromAzcoreError(err error) (bool, error) {
if err == nil {
return true, nil
}
var respError *azcore.ResponseError
if errors.As(err, &respError) && respError != nil {
if respError.StatusCode == http.StatusNotFound {
return false, nil
}
}

return false, err
}

func (az *Cloud) useStandardLoadBalancer() bool {
return strings.EqualFold(az.LoadBalancerSku, consts.LoadBalancerSkuStandard)
}
Expand Down
6 changes: 3 additions & 3 deletions pkg/provider/loadbalancer/accesscontrol.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,9 @@ import (
"k8s.io/utils/ptr"

"sigs.k8s.io/cloud-provider-azure/pkg/consts"
"sigs.k8s.io/cloud-provider-azure/pkg/provider/loadbalancer/fnutil"
"sigs.k8s.io/cloud-provider-azure/pkg/provider/loadbalancer/iputil"
"sigs.k8s.io/cloud-provider-azure/pkg/provider/loadbalancer/securitygroup"
"sigs.k8s.io/cloud-provider-azure/pkg/provider/securitygroup"
fnutil "sigs.k8s.io/cloud-provider-azure/pkg/util/collectionutil"
"sigs.k8s.io/cloud-provider-azure/pkg/util/iputil"
)

var (
Expand Down
6 changes: 3 additions & 3 deletions pkg/provider/loadbalancer/accesscontrol_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,9 @@ import (
"sigs.k8s.io/cloud-provider-azure/internal/testutil"
"sigs.k8s.io/cloud-provider-azure/internal/testutil/fixture"
"sigs.k8s.io/cloud-provider-azure/pkg/log"
"sigs.k8s.io/cloud-provider-azure/pkg/provider/loadbalancer/fnutil"
"sigs.k8s.io/cloud-provider-azure/pkg/provider/loadbalancer/iputil"
"sigs.k8s.io/cloud-provider-azure/pkg/provider/loadbalancer/securitygroup"
"sigs.k8s.io/cloud-provider-azure/pkg/provider/securitygroup"
fnutil "sigs.k8s.io/cloud-provider-azure/pkg/util/collectionutil"
"sigs.k8s.io/cloud-provider-azure/pkg/util/iputil"
)

func TestAccessControl_IsAllowFromInternet(t *testing.T) {
Expand Down
2 changes: 1 addition & 1 deletion pkg/provider/loadbalancer/configuration.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import (
v1 "k8s.io/api/core/v1"

"sigs.k8s.io/cloud-provider-azure/pkg/consts"
"sigs.k8s.io/cloud-provider-azure/pkg/provider/loadbalancer/iputil"
"sigs.k8s.io/cloud-provider-azure/pkg/util/iputil"
)

// IsInternal returns true if the given service is internal load balancer.
Expand Down
102 changes: 60 additions & 42 deletions pkg/provider/azure_securitygroup_repo.go → ...securitygroup/azure_securitygroup_repo.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
limitations under the License.
*/

package provider
package securitygroup

import (
"context"
Expand All @@ -31,20 +31,66 @@ import (
"k8s.io/klog/v2"
"k8s.io/utils/ptr"

"sigs.k8s.io/cloud-provider-azure/pkg/azclient/securitygroupclient"
azcache "sigs.k8s.io/cloud-provider-azure/pkg/cache"
"sigs.k8s.io/cloud-provider-azure/pkg/consts"
"sigs.k8s.io/cloud-provider-azure/pkg/util/errutils"
)

// CreateOrUpdateSecurityGroup invokes az.SecurityGroupsClient.CreateOrUpdate with exponential backoff retry
func (az *Cloud) CreateOrUpdateSecurityGroup(sg *armnetwork.SecurityGroup) error {
ctx, cancel := getContextWithCancel()
defer cancel()
clientFactory := az.NetworkClientFactory
if clientFactory == nil {
clientFactory = az.ComputeClientFactory
const (
nsgCacheTTLDefaultInSeconds = 120
)

type Repository interface {
GetSecurityGroup(ctx context.Context) (*armnetwork.SecurityGroup, error)
CreateOrUpdateSecurityGroup(ctx context.Context, sg *armnetwork.SecurityGroup) error
}

type securityGroupRepo struct {
securityGroupResourceGroup string
securityGroupName string
nsgCacheTTLInSeconds int
securigyGroupClient securitygroupclient.Interface
nsgCache azcache.Resource
}

func NewSecurityGroupRepo(securityGroupResourceGroup string, securityGroupName string, nsgCacheTTLInSeconds int, disableAPICallCache bool, securityGroupClient securitygroupclient.Interface) (Repository, error) {
getter := func(ctx context.Context, key string) (interface{}, error) {
nsg, err := securityGroupClient.Get(ctx, securityGroupResourceGroup, key)
exists, rerr := errutils.CheckResourceExistsFromAzcoreError(err)
if rerr != nil {
return nil, err
}

if !exists {
klog.V(2).Infof("Security group %q not found", key)
return nil, nil
}

return nsg, nil
}

if nsgCacheTTLInSeconds == 0 {
nsgCacheTTLInSeconds = nsgCacheTTLDefaultInSeconds
}
cache, err := azcache.NewTimedCache(time.Duration(nsgCacheTTLInSeconds)*time.Second, getter, disableAPICallCache)
if err != nil {
klog.Errorf("Failed to create cache for security group %q: %v", securityGroupName, err)
return nil, err
}
sgClient := clientFactory.GetSecurityGroupClient()
_, rerr := sgClient.CreateOrUpdate(ctx, az.SecurityGroupResourceGroup, *sg.Name, *sg)

return &securityGroupRepo{
securityGroupResourceGroup: securityGroupResourceGroup,
securityGroupName: securityGroupName,
nsgCacheTTLInSeconds: nsgCacheTTLDefaultInSeconds,
securigyGroupClient: securityGroupClient,
nsgCache: cache,
}, nil
}

// CreateOrUpdateSecurityGroup invokes az.SecurityGroupsClient.CreateOrUpdate with exponential backoff retry
func (az *securityGroupRepo) CreateOrUpdateSecurityGroup(ctx context.Context, sg *armnetwork.SecurityGroup) error {
_, rerr := az.securigyGroupClient.CreateOrUpdate(ctx, az.securityGroupResourceGroup, *sg.Name, *sg)
klog.V(10).Infof("SecurityGroupsClient.CreateOrUpdate(%s): end", *sg.Name)
if rerr == nil {
// Invalidate the cache right after updating
Expand All @@ -71,47 +117,19 @@ func (az *Cloud) CreateOrUpdateSecurityGroup(sg *armnetwork.SecurityGroup) error
return rerr
}

func (az *Cloud) newNSGCache() (azcache.Resource, error) {
getter := func(ctx context.Context, key string) (interface{}, error) {
clientFactory := az.NetworkClientFactory
if clientFactory == nil {
clientFactory = az.ComputeClientFactory
}
sgClient := clientFactory.GetSecurityGroupClient()

nsg, err := sgClient.Get(ctx, az.SecurityGroupResourceGroup, key)
exists, rerr := checkResourceExistsFromAzcoreError(err)
if rerr != nil {
return nil, err
}

if !exists {
klog.V(2).Infof("Security group %q not found", key)
return nil, nil
}

return nsg, nil
}

if az.NsgCacheTTLInSeconds == 0 {
az.NsgCacheTTLInSeconds = nsgCacheTTLDefaultInSeconds
}
return azcache.NewTimedCache(time.Duration(az.NsgCacheTTLInSeconds)*time.Second, getter, az.Config.DisableAPICallCache)
}

func (az *Cloud) getSecurityGroup(ctx context.Context, crt azcache.AzureCacheReadType) (*armnetwork.SecurityGroup, error) {
func (az *securityGroupRepo) GetSecurityGroup(ctx context.Context) (*armnetwork.SecurityGroup, error) {
nsg := &armnetwork.SecurityGroup{}
if az.SecurityGroupName == "" {
if az.securityGroupName == "" {
return nsg, fmt.Errorf("securityGroupName is not configured")
}

securityGroup, err := az.nsgCache.GetWithDeepCopy(ctx, az.SecurityGroupName, crt)
securityGroup, err := az.nsgCache.GetWithDeepCopy(ctx, az.securityGroupName, azcache.CacheReadTypeDefault)
if err != nil {
return nsg, err
}

if securityGroup == nil {
return nsg, fmt.Errorf("nsg %q not found", az.SecurityGroupName)
return nsg, fmt.Errorf("nsg %q not found", az.securityGroupName)
}

return securityGroup.(*armnetwork.SecurityGroup), nil
Expand Down
Loading

0 comments on commit d0b7bcb

Please sign in to comment.