Hello everyone! My name is Youssef and I am workin...
# contributors
s
Hello everyone! My name is Youssef and I am working along side my colleague Achraf at Amadeus and our goal is to add ARM/Bicep support into Infracost. So far, we were able to accomplish that, and we are reaching out so we could get your feedback regarding it However, we don't want to contribute this solution directly. While working on adding this support, and learning more about the resource generation code and the current PRs, we noticed that there are cases of code duplication occurring inside the
providers
directory. For example, in Terraform and CF, the AWS code for DynamoDB seems to be doing pretty much the same thing, which is the extraction of information from the resource data. Another example where we noticed this case of duplication is in the repository that is focused on adding support for Pulumi. The only thing that is different between all these functions is the string being passed to the Get function (CF isn't using the Get function, but it could be modified to do so) (https://github.com/rshade/infracost/blob/feat/pulumi/internal/providers/pulumi/aws/dynamodb_table.go) Therefore, we figured we could probably try to refactor the code first to reduce duplication and implementation, and increase scalability when it comes to adding new IaC tools. We would love to have a discussion with you regarding our solution and get your feedback before we open the PR, to ensure we go it the right way.
m
Hi @swift-rocket-90720, I’m a little confused by what your asking. I’d love to clarify some things:
However, we don’t want to contribute this solution directly.
So are you adding ARM support to a private fork or something you will maintain yourself?
For example, in Terraform and CF, the AWS code for DynamoDB seems to be doing pretty much the same thing, which is the extraction of information from the resource data.
Yes that is correct the providers logic is essentially a mapping layer
The only thing that is different between all these functions is the string being passed to the Get function (CF isn’t using the Get function, but it could be modified to do so)
In general we have not focused on generalising the providers layer because we are fully focused on Terraform support for now. So it has not been a priority to refactor/rework the mapping layer as we only really support Terraform (cloudformation is only barely supported)
We would love to have a discussion with you regarding our solution and get your feedback before we open the PR.
I think the best way to discuss this is probably via a PR to see what you propose.
s
So are you adding ARM support to a private fork or something you will maintain yourself?
Initially, we did the refactoring part first, then implemented support for ARM. However, we noticed that the PR would be too big as the refactoring part has a lot of file location changes. Therefore, we decided to split this contribution into two separate PRs to make it easier to read and understand.
In general we have not focused on generalising the providers layer because we are fully focused on Terraform support for now. So it has not been a priority to refactor/rework the mapping layer as we only really support Terraform (cloudformation is only barely supported)
Our change focuses on generalizing the providers layer, as we have been able to implement a few resources in ARM without having to re-implement the same logic elsewhere for example. There are quite a few details regarding this part, and we are preparing a document that explains the general idea of what we have changed
I think the best way to discuss this is probably via a PR to see what you propose.
At the moment, we are not able to provide you with a PR as we are still preparing and finalizing. However, we can provide you with a document, after finalizing it, that explains what we are proposing.
m
Ok I think I understand better now. So, firstly, I’d definitely love to read and understand your proposal for generalising the providers. However I just want to set expectations. The provider abstraction and resource logic is something core to Infracost and our wider services. I think any refactor around how providers work would need to be handled by the core team. It’s something that needs to fit into our wider roadmap and how we handle things with Infracost Cloud. We’re working on this area at the moment and, whilst this work is not in advanced stages, is something that we want to get right and take time to think about. So it’s unlikely that we’d accept any open source contributions in this area until we’ve at least formalised some of our thinking into a general consensus amongst the team and our customers. So that being said, I would probably pause any active work you are doing at the moment to try add such refactors/support to Infracost CLI. Sorry, this probably isn’t the news that you want to hear, but this area is something extremely important to us and our future.
s
Yes, we would love to share the proposal with you and get your opinion on it. Although at the moment, you are not accepting any major contributions that could impact your code base, we would love to propose it anyways, and perhaps talk about different steps to approach the ARM support PR. I will contact you on Monday and provide you with the document that further explains what we did 🙂
m
sounds good
s
Hello, please find attached our proposal 🙂
m
sorry for the slow reply youssef, I was on holiday last week
let me review this with the team this week
👍 1
s
Hello No problem 🙂