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.

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

Add aks support #100

Merged
merged 14 commits into from
Dec 5, 2024
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
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
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,4 @@
# vendor/

cosign.*
.idea
3 changes: 2 additions & 1 deletion Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@ RUN addgroup -S k8s-pvc-tagger && adduser -S k8s-pvc-tagger -G k8s-pvc-tagger
# Build a small image
FROM scratch
COPY --from=builder /etc/passwd /etc/passwd
USER k8s-pvc-tagger
boris-smidt-klarrio marked this conversation as resolved.
Show resolved Hide resolved
ENV APP_NAME=k8s-pvc-tagger

# https://github.com/aws/aws-sdk-go/issues/2322
COPY --from=builder /etc/ssl/certs/ca-certificates.crt /etc/ssl/certs/
COPY --from=builder /app/${APP_NAME} /
Expand Down
5 changes: 4 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ metadata:

### Multi-cloud support

Currently supported clouds: AWS, GCP.
Currently supported clouds: AWS, GCP, Azure

Only one mode is active at a given time. Specify the cloud `k8s-pvc-tagger` is running in with the `--cloud` flag. Either `aws` or `gcp`.

Expand Down Expand Up @@ -117,6 +117,9 @@ gcloud iam roles create CustomDiskRole \
--stage="GA"
```

#### Azure rule
The default role `Tag Contributor` can be used to configure the access rights for the pvc-tagger.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a bit about how this only support Azure disk csi volumes?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've documented the behaviors hopefully this description will stay up to date in the future.


#### Install via helm

```
Expand Down
173 changes: 173 additions & 0 deletions aks.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,173 @@
package main
boris-smidt-klarrio marked this conversation as resolved.
Show resolved Hide resolved

import (
"context"
"errors"
"fmt"
"github.com/Azure/azure-sdk-for-go/sdk/azcore/arm"
"github.com/Azure/azure-sdk-for-go/sdk/azcore/to"
"github.com/Azure/azure-sdk-for-go/sdk/azidentity"
"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armresources"
"github.com/prometheus/client_golang/prometheus"
log "github.com/sirupsen/logrus"
"maps"
"strings"
)

var (
ErrAzureTooManyTags error = errors.New("Only up to 50 tags can be set on an azure resource")
ErrAzureValueToLong error = errors.New("A value can only contain 256 characters")
)

type DiskTags = map[string]*string
type AzureSubscription = string

type AzureClient interface {
GetDiskTags(ctx context.Context, subscription AzureSubscription, resourceGroupName string, diskName string) (DiskTags, error)
SetDiskTags(ctx context.Context, subscription AzureSubscription, resourceGroupName string, diskName string, tags DiskTags) error
}

type azureClient struct {
client *armresources.TagsClient
}

func NewAzureClient() (AzureClient, error) {
creds, err := azidentity.NewDefaultAzureCredential(nil)
if err != nil {
return nil, err
}
client, err := armresources.NewTagsClient("", creds, &arm.ClientOptions{})
if err != nil {
return nil, err
}

return azureClient{client}, err
}

func diskScope(subscription string, resourceGroupName string, diskName string) string {
return fmt.Sprintf("subscriptions/%s/resourceGroups/%s/providers/Microsoft.Compute/disks/%s", subscription, resourceGroupName, diskName)
}

func (self azureClient) GetDiskTags(ctx context.Context, subscription AzureSubscription, resourceGroupName string, diskName string) (DiskTags, error) {

tags, err := self.client.GetAtScope(ctx, diskScope(subscription, resourceGroupName, diskName), &armresources.TagsClientGetAtScopeOptions{})
if err != nil {
return nil, fmt.Errorf("could not get the tags for: %w", err)
}

return tags.Properties.Tags, nil
}

func (self azureClient) SetDiskTags(ctx context.Context, subscription AzureSubscription, resourceGroupName string, diskName string, tags DiskTags) error {
response, err := self.client.UpdateAtScope(
ctx,
diskScope(subscription, resourceGroupName, diskName),
armresources.TagsPatchResource{
to.Ptr(armresources.TagsPatchOperationReplace),
&armresources.Tags{Tags: tags},
}, &armresources.TagsClientUpdateAtScopeOptions{},
)
if err != nil {
return fmt.Errorf("could not set the tags for: %w", err)
}
log.WithFields(log.Fields{"disk": diskName, "resource-group": resourceGroupName}).Debugf("updated disk tags to tags=%v", response.Properties.Tags)
return nil
}

func parseAzureVolumeID(volumeID string) (subscription string, resourceGroup string, diskName string, err error) {
// '/subscriptions/{subscription}/resourceGroups/{resourceGroup}/providers/Microsoft.Compute/disks/{diskname}"'
fields := strings.Split(volumeID, "/")
if len(fields) != 9 {
return "", "", "", errors.New("invalid volume id")
}
subscription = fields[2]
resourceGroup = fields[4]
diskName = fields[8]
return subscription, resourceGroup, diskName, nil
}

func sanitizeLabelsForAzure(tags map[string]string) (DiskTags, error) {
boris-smidt-klarrio marked this conversation as resolved.
Show resolved Hide resolved
diskTags := make(DiskTags)
if len(tags) > 50 {
return nil, ErrAzureTooManyTags
}
for k, v := range tags {
k = sanitizeKeyForAzure(k)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After sanitizing the key it is possible that it could end up being a duplicate key. I'm on the fence as to whether it would be better to reject the tag if the key needs to be sanitized or if the tag created a duplicate. Which approach do you think is better?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We use this in AWS and Azure so we don't really want to change the tags depending on the envrionment.
But i will add an error in case there are duplicated keys, this should also tackle your other comment concerning Kubernetes/Cluster: foo

value, err := sanitizeValueForAzure(v)
if err != nil {
return nil, err
}

diskTags[k] = &value
}

return diskTags, nil
}

func sanitizeKeyForAzure(s string) string {
// remove forbidden characters
if strings.ContainsAny(s, `<>%&\?/`) {
for _, c := range `<>%&\?/` {
s = strings.ReplaceAll(s, string(c), "")
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the invalid characters are being removed from the key it is possible to bypass this logic https://github.com/mtougeron/k8s-pvc-tagger/blob/main/kubernetes.go#L327-L339 by creating a tag name of Kubernetes/Cluster: foo

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its solved by rejecting duplicated tag keys during the sanitization.

}
}

// truncate the key the max length for azure
if len(s) > 512 {
s = s[:512]
}

return s
}

func sanitizeValueForAzure(s string) (string, error) {
// the value can contain at most 256 characters
if len(s) > 256 {
return "", fmt.Errorf("%s value is invalid: %w", s, ErrAzureValueToLong)
}
return s, nil
}

func UpdateAzureVolumeTags(ctx context.Context, client AzureClient, volumeID string, tags map[string]string, removedTags []string, storageclass string) error {
sanitizedLabels, err := sanitizeLabelsForAzure(tags)
if err != nil {
return err
}

log.Debugf("labels to add to PD volume: %s: %v", volumeID, sanitizedLabels)
subscription, resourceGroup, diskName, err := parseAzureVolumeID(volumeID)
if err != nil {
return err
}

existingTags, err := client.GetDiskTags(ctx, subscription, resourceGroup, diskName)
if err != nil {
return err
}

// merge existing disk labels with new labels:
updatedTags := make(DiskTags)
if existingTags != nil {
updatedTags = maps.Clone(existingTags)
}
maps.Copy(updatedTags, sanitizedLabels)

for _, tag := range removedTags {
delete(updatedTags, tag)
}

if maps.Equal(existingTags, updatedTags) {
log.Debug("labels already set on PD")
return nil
}

err = client.SetDiskTags(ctx, subscription, resourceGroup, diskName, updatedTags)
if err != nil {
promActionsTotal.With(prometheus.Labels{"status": "error", "storageclass": storageclass}).Inc()
return err
}

log.Debug("successfully set labels on PD")
promActionsTotal.With(prometheus.Labels{"status": "success", "storageclass": storageclass}).Inc()
return nil
}
176 changes: 176 additions & 0 deletions aks_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,176 @@
package main

import (
"fmt"
"github.com/stretchr/testify/assert"
"strings"
"testing"
)

func Test_parseAzureVolumeID(t *testing.T) {
type args struct {
volumeID string
}
tests := []struct {
name string
args args
wantSubscription string
wantResourceGroup string
wantDiskName string
wantErr bool
}{
{
name: "test using a correct volume ID",
args: args{volumeID: "/subscriptions/{subscription}/resourceGroups/{resourceGroup}/providers/Microsoft.Compute/disks/{diskname}"},
wantSubscription: "{subscription}",
wantResourceGroup: "{resourceGroup}",
wantDiskName: "{diskname}",
wantErr: false,
},
{
name: "test using a correct volume ID",
args: args{volumeID: "/subscriptions/{subscription}/resourceGroups/{resourceGroup}/providers/Microsoft.Compute/disks"},
wantSubscription: "",
wantResourceGroup: "",
wantDiskName: "",
wantErr: true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
gotSubscription, gotResourceGroup, gotDiskName, err := parseAzureVolumeID(tt.args.volumeID)
if (err != nil) != tt.wantErr {
t.Errorf("parseAzureVolumeID() error = %v, wantErr %v", err, tt.wantErr)
return
}
if gotSubscription != tt.wantSubscription {
t.Errorf("parseAzureVolumeID() gotSubscription = %v, want %v", gotSubscription, tt.wantSubscription)
}
if gotResourceGroup != tt.wantResourceGroup {
t.Errorf("parseAzureVolumeID() gotResourceGroup = %v, want %v", gotResourceGroup, tt.wantResourceGroup)
}
if gotDiskName != tt.wantDiskName {
t.Errorf("parseAzureVolumeID() gotDiskName = %v, want %v", gotDiskName, tt.wantDiskName)
}
})
}
}

func Test_sanitizeKeyForAzure(t *testing.T) {
type args struct {
s string
}
var tests = []struct {
name string
args args
want string
}{
{
name: "the key should be trimmed to 512 characters",
args: args{
s: strings.Repeat("1", 513),
},
want: strings.Repeat("1", 512),
},
{
name: "the key should remove all invalid characters",
args: args{
s: `1<>&\?%/`,
},
want: "1",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
if got := sanitizeKeyForAzure(tt.args.s); got != tt.want {
t.Errorf("sanitizeKeyForAzure() = %v, want %v", got, tt.want)
}
})
}
}

func Test_sanitizeValueForAzure(t *testing.T) {
type args struct {
s string
}
tests := []struct {
name string
args args
want string
wantErr bool
}{
{
name: "a valid value",
args: args{strings.Repeat("1", 256)},
want: strings.Repeat("1", 256),
wantErr: false,
},

{
name: "the max value lenght is 256 characters",
args: args{strings.Repeat("1", 257)},
want: "",
wantErr: true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
got, err := sanitizeValueForAzure(tt.args.s)
if (err != nil) != tt.wantErr {
t.Errorf("sanitizeValueForAzure() error = %v, wantErr %v", err, tt.wantErr)
return
}
if got != tt.want {
t.Errorf("sanitizeValueForAzure() got = %v, want %v", got, tt.want)
}
})
}
}

func Test_sanitizeLabelsForAzure(t *testing.T) {
t.Run("azure supports up to 50 labels", func(t *testing.T) {
t.Parallel()
tags := map[string]string{}
for x := 0; x < 50; x++ {
v := fmt.Sprintf("%d", x)
tags[v] = v
}
_, err := sanitizeLabelsForAzure(tags)
assert.NoError(t, err)

tags["51"] = "51"
_, err = sanitizeLabelsForAzure(tags)
assert.ErrorIs(t, err, ErrAzureTooManyTags)
})
}

func Test_diskScope(t *testing.T) {
type args struct {
subscription string
resourceGroupName string
diskName string
}
tests := []struct {
name string
args args
want string
}{
{
"it should generate a valid scope for disks",
args{
subscription: "sub",
resourceGroupName: "resource-name",
diskName: "disk-name",
},
"subscriptions/sub/resourceGroups/resource-name/providers/Microsoft.Compute/disks/disk-name",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
assert.Equalf(t, tt.want, diskScope(tt.args.subscription, tt.args.resourceGroupName, tt.args.diskName), "diskScope(%v, %v, %v)", tt.args.subscription, tt.args.resourceGroupName, tt.args.diskName)
})
}
}
Loading
Loading