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 support for using an Azure storage account backend #1736

Conversation

NeverOddOrEven
Copy link

I've added support for using an Azure storage account on the backend. I mostly copied the GCS implementation and then retrofitted in what I believe to be the right code from the Azure Golang SDK.

While I've written plenty of code in many other languages, this is the first Golang code I've written that isn't a basic Hello World web app. I would appreciate any helpful feedback or suggestions.

@NeverOddOrEven
Copy link
Author

It seems like this would be useful for those of us building solutions on Azure. Any feedback? I would be happy to address any problems and add documentation if this PR has a viable path to being merged. Let me know! Thanks.

Copy link
Contributor

@yorinasub17 yorinasub17 left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution and apologies for the delay in reviewing. We internally don't have a lot of Azure expertise so it takes some time for us to review a feature like this.

A surface level pass at the code looks good to me. However, I see two things missing:

That said, I think the biggest concern for us for merging this in would be that we don't have an Azure account where we can have integration tests for this on a continuing basis. We will need to discuss internally on how to address that.

@NeverOddOrEven
Copy link
Author

Thanks for the heads up. I think all of those points are very addressable. I should have time this weekend to add the documentation and integration tests. If you would like a volunteer consult on the Azure account aspect, let me know. I would be happy to advise/help. While cloud spend would be negligible, I will see if I can organize some information relating to Azure and OSS.

@coip
Copy link

coip commented Jul 16, 2021

You might consider externalizing the hard-coded ctx = context.Background()s to the caller and instead require ctx context.Context as the first param for the associated funcs.

ctx is by convention always the first argument to a function that propagates context. Neat go primitive.

https://pkg.go.dev/context

very nice tho! Exciting to see

@BasLangenberg
Copy link

Hi, while looking to add support for Azure storage account backends, I found this PR already open! Is there any chance this will be merged?

@NeverOddOrEven
Copy link
Author

I had hoped to make the suggested changes to get this merged. However, I have since changed jobs and I don’t think I am able to carry this across the finish line. I would love to see someone else pick it up if they are able!

Fwiw, it is working code. It just needs merge conflicts resolved and integration test coverage.

@BasLangenberg
Copy link

I'll have a stab at getting this in mergable state. I'm pretty busy right now, but I expect to have time for this in a week or 2.

@skurhse-rage-nexient
Copy link

Any update on this?

@adamdost-0
Copy link

@yorinasub17 is the issue that there is no Azure environment for you all to test in?

@DanielMabbett
Copy link

Hi @yorinasub17 how is this going? Its been a year and a half

@DanielMabbett
Copy link

Just for update asked in the discussion area. Keen to see this feature :)
gruntwork-io/knowledge-base#648

@brikis98
Copy link
Member

brikis98 commented Feb 1, 2023

Folks, apologies for the long delay here. We would love to add first-class support for Azure backends, but the truth is that our team at Gruntwork just doesn't have the capacity to do it right now. As a company, we're mostly focused on AWS these days, so supporting Azure is hard: we don't have that much Azure expertise; we don't have Azure accounts set up to run tests; we don't have a tool like cloud-nuke set up for Azure to clean up those accounts (so tests don't cost us a fortune); and so on. And, of course, we don't want to blindly merge a feature with no expertise or testing around it, so it looks like we support it, but it doesn't actually work. So, for the time being, I'm going to close this PR, unmerged.

We are deeply grateful for the community around Terragrunt; it's wonderful to see the ways you're all using this tool and all of your contributions have been invaluable. We wish we could do a lot more here, but as a small, 100% bootstrapped company maintaining a lot of open source software, we're doing the best we can to make things work (we're even experimenting with sponsorships!). Feedback and suggestions are very welcome.

@brikis98 brikis98 closed this Feb 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants