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

Jump to bottom

make azure storage account private #255

Open
wants to merge 3 commits into
base: refacto-and-prep-for-private-storage-account
Choose a base branch
from
Open

make azure storage account private #255

wants to merge 3 commits into from

Conversation

QuentinBisson
Copy link
Contributor

@QuentinBisson QuentinBisson commented Nov 28, 2024 •
edited
Loading

What this PR does / why we need it

Towards giantswarm/roadmap#3391

This PR actually allows us to create private only azure storage accounts in case we are running MCs in private mode.

To achieve that, this PR:

  • detects whever the management cluster is running in private mode (by reading the mc user-values) as this is the only place that contains this information
  • disallow the public access of the bucket for private buckets only so gaggle will be unaffected
  • creae a private endpoint and all the necessary network resources (private dns zone, virtual network link to attach the endpoint to the dns zone, a dns A records to point to the private endpoint in the private zone) to make this all work.

This PR is based on https://github.com/giantswarm/object-storage-operator/tree/refacto-and-prep-for-private-storage-account to make review easier.

Checklist

  • Update changelog in CHANGELOG.md.

@QuentinBisson QuentinBisson self-assigned this Nov 28, 2024
@QuentinBisson QuentinBisson force-pushed the make-azure-storage-accounts-private branch 9 times, most recently from 229d7cd to d59b29f Compare December 2, 2024 14:47
QuentinBisson
QuentinBisson commented Dec 2, 2024
View reviewed changes
internal/controller/bucket_controller.go Outdated
@@ -110,7 +110,12 @@ func (r BucketReconciler) reconcileNormal(ctx context.Context, objectStorageServ
return ctrl.Result{}, errors.WithStack(err)
}
} else {
logger.Info("Bucket exists and you already own it.")
logger.Info("Bucket exists and you already own it, let's update it")
err = objectStorageService.UpdateBucket(ctx, bucket)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unless we want to delete all our storage accounts, we need to be able to update azure storage

QuentinBisson
QuentinBisson commented Dec 2, 2024
View reviewed changes
internal/pkg/service/objectstorage/cloud/azure/storage.go Outdated
func (s *AzureObjectStorageAdapter) getStorageAccountName(bucketName string) string {
sanitizeName := sanitizeAlphanumeric24(bucketName)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This had no use

@QuentinBisson QuentinBisson force-pushed the make-azure-storage-accounts-private branch 18 times, most recently from 6257228 to 4448883 Compare December 5, 2024 11:13
@QuentinBisson QuentinBisson force-pushed the make-azure-storage-accounts-private branch 7 times, most recently from 4b3b09d to 25fd866 Compare December 5, 2024 15:36
QuentinBisson
QuentinBisson commented Dec 5, 2024
View reviewed changes
CHANGELOG.md Outdated Show resolved Hide resolved
QuentinBisson
QuentinBisson commented Dec 5, 2024
View reviewed changes
internal/pkg/service/objectstorage/cloud/aws/s3.go Outdated
@@ -64,6 +64,11 @@ func (s S3ObjectStorageAdapter) CreateBucket(ctx context.Context, bucket *v1alph
return err
}

// UpdateBucket does nothing as we cannot update an s3 bucket
func (s S3ObjectStorageAdapter) UpdateBucket(ctx context.Context, bucket *v1alpha1.Bucket) error {
return nil
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 need to be able to update storage accounts in Azure. I'm not trying to implement this in AWS because this is not needed now and also well, this will be replaced by crossplane

QuentinBisson
QuentinBisson commented Dec 5, 2024
View reviewed changes
internal/pkg/service/objectstorage/cloud/azure/container.go Outdated
"github.com/giantswarm/object-storage-operator/api/v1alpha1"
)

func (s AzureObjectStorageAdapter) existsContainer(ctx context.Context, bucket *v1alpha1.Bucket, storageAccountName string) (bool, error) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is not a new function but I split the existing functions from storage.go into smaller files to be more manageable

@QuentinBisson QuentinBisson marked this pull request as ready for review December 5, 2024 17:58
@QuentinBisson QuentinBisson requested a review from a team as a code owner December 5, 2024 17:58
@QuentinBisson QuentinBisson changed the base branch from main to refacto-and-prep-for-private-storage-account December 5, 2024 18:19
@QuentinBisson QuentinBisson changed the base branch from refacto-and-prep-for-private-storage-account to main December 5, 2024 18:32
@QuentinBisson QuentinBisson changed the base branch from main to refacto-and-prep-for-private-storage-account December 5, 2024 18:33
@QuentinBisson QuentinBisson force-pushed the make-azure-storage-accounts-private branch from 35319cc to 4dfa6f6 Compare December 5, 2024 18:36
@QuentinBisson QuentinBisson force-pushed the make-azure-storage-accounts-private branch from 4dfa6f6 to 795725e Compare December 5, 2024 18:39
@QuentinBisson QuentinBisson force-pushed the make-azure-storage-accounts-private branch 4 times, most recently from 034a54d to 83f2ca2 Compare December 9, 2024 16:13
@QuentinBisson QuentinBisson force-pushed the make-azure-storage-accounts-private branch from 83f2ca2 to 1aaf5df Compare December 9, 2024 16:30
@QuentinBisson QuentinBisson mentioned this pull request Dec 9, 2024
Open
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@QuentinBisson QuentinBisson

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@QuentinBisson