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

Adding hack for allowSharedKeyAccess = false on storage accounts #3595

Closed
wants to merge 1 commit into from
Closed

Adding hack for allowSharedKeyAccess = false on storage accounts #3595

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion pkg/deploy/assets/rp-production.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -541,7 +541,7 @@
},
"location": "[resourceGroup().location]",
"name": "[substring(parameters('storageAccountDomain'), 0, indexOf(parameters('storageAccountDomain'), '.'))]",
"type": "Microsoft.Storage/storageAccounts",
"type": "Microsoft.Storage/storageAccounts", { "properties": "allowSharedAccessKey: false" },
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note to self: I can add \n to make this better indented. This was just a proof of concept / draft.

"apiVersion": "2019-06-01"
},
{
Expand Down
8 changes: 7 additions & 1 deletion pkg/deploy/generator/generators.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,14 @@
return nil
}

func (g *generator) writeTemplate(t *arm.Template, output string) error {

Check failure on line 116 in pkg/deploy/generator/generators.go

View workflow job for this annotation

GitHub Actions / golangci-lint 

unnecessary leading newline (whitespace)
b, err := g.templateFixup(t)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note to self: fix this due to the linter failure.


sharedAccessKeyHack := false
if output == FileRPProduction {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

For Nont and Weinong:

Without this if, some other files besides rp-production.json get modified, such as rp-oicd.json, among others.

As I am not sure yet which set of the files we want to modify, I added this to help us control that.

I'm thinking probably all files, as we don't want any more SAS keys anywhere. But, still, being able to control it feels safer - I don't know if all the storage accounts have other authentication ways and are ready for SAS to be disabled.

sharedAccessKeyHack = true
}

b, err := g.templateFixup(t, sharedAccessKeyHack)
if err != nil {
return err
}
Expand Down
28 changes: 27 additions & 1 deletion pkg/deploy/generator/templates.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ func max(is ...int) int {
return max
}

func (g *generator) templateFixup(t *arm.Template) ([]byte, error) {
func (g *generator) templateFixup(t *arm.Template, sharedAccessKeyHack bool) ([]byte, error) {
b, err := json.MarshalIndent(t, "", " ")
if err != nil {
return nil, err
Expand Down Expand Up @@ -68,6 +68,32 @@ func (g *generator) templateFixup(t *arm.Template) ([]byte, error) {
b = bytes.Replace(b, []byte(`"sourceAddressPrefixes": []`), []byte(`"sourceAddressPrefixes": "[parameters('rpNsgPortalSourceAddressPrefixes')]"`), 1)
}

if sharedAccessKeyHack {
b = bytes.ReplaceAll(b, []byte(`"type": "Microsoft.Storage/storageAccounts"`), []byte(`"type": "Microsoft.Storage/storageAccounts", { "properties": "allowSharedAccessKey: false" }`))
}

// TO-DO:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

For Weinong and Nont:

Added this so that we don't forget to consider doing the SDK migration instead of keeping this hack here for too long.

Also added a mention to this on the SDK migration wiki itself: https://msazure.visualstudio.com/AzureRedHatOpenShift/_wiki/wikis/AzureRedHatOpenShift.wiki/604062/Azure-SDK-track-2-migration

// This hack allows us to specify `allowSharedKeyAccess = false` for the storage accounts.
// This is required by Security Wave - 2024.
// However, the reason this hack is necessary is that we are using the old package for ARM Template Storage Accounts (mgmt/storage).
// If we complete the migration from SDK track 1 to SDK track 2, which includes using armstorage instead of mgmt/storage,
// we are able to directly set allowStorageKeyAccess to false in the ARM template struct itself,
// specifically on resource_rp.go's rpStorageAccount() function or g.storageAccount() from resources.go.
// When we do this and start setting allowSharedKeyAccess to false directly on the functions, this hack can be safely removed.

// Example usage of new package in AKS: https://msazure.visualstudio.com/CloudNativeCompute/_git/aksiknife?path=/vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/storage/armstorage/models.go&version=GBmaster&line=197&lineEnd=197&lineStartColumn=1&lineEndColumn=68&lineStyle=plain&_a=contents

// New package docs: https://pkg.go.dev/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/storage/armstorage

// Package migration guide: https://github.com/Azure/azure-sdk-for-go/blob/main/documentation/MIGRATION_GUIDE.md

// Old package, latest version, containing deprecation warning: https://pkg.go.dev/github.com/Azure/azure-sdk-for-go/services/storage/mgmt/2019-06-01/storage

// Old package, old version, which we're using, so it took me a while to find the deprecation warning: https://pkg.go.dev/github.com/Azure/[email protected]+incompatible/services/storage/mgmt/2019-06-01/storage

// ARM Template modification: add "properties:" {"allowSharedKeyAccess": false} to the storage account resource.
// As described by ARM Template documentation, here: https://learn.microsoft.com/en-us/azure/templates/microsoft.storage/storageaccounts?pivots=deployment-language-arm-template

return append(b, byte('\n')), nil
}

Expand Down
Loading