carvel-dev / carvel-dev/kapp

diff internals: Introducing a DiffResource(s) struct

Open
#635 4 comments 0 reactions 1 assignee Claimed by @criztovyl View on GitHub
carvel accepted enhancement
Dominant language
Go
Stars
1.1k
Forks
134
PR merge metrics
No merged PRs in 30d

Description

There are some methods in `changeset_with_versioned_rs.go` that could qualify for a dedicated type/struct:

> I think I would prefer it if this function [`newGroupedVersionedResources`] and the next one [`existingResourcesMap`] are either scoped to a struct, maybe we can move stuff that are used by other parts of the code to a single place at a later point. Not a blocker but a nice to have

_Originally posted by @100mik in https://github.com/vmware-tanzu/carvel-kapp/pull/571#discussion_r989791601_

The changes introduced in #571 actually introduced a conflict with "far-reaching" consequences to my #517 PR.
(71 and 17, what a coincidence!)

I thought about the following there, what do you think about it?
I would try to implement that in a dedicated PR.

Note that My PR also introduces an abstraction for `VersionedResource`, but i never was quite happy with that name, maybe that could be lifted up into a `DiffResource`, extracting it from my PR into the new/dedicated PR?

> #571 introduced a conflict on `func (d ChangeSetWithVersionedRs) groupResources([]ctlres.Resource)`: it moved it to `func newGroupedVersionedResources(...)` and uses it via `diff.existingResourcesMap`. But I changed the method to use (and return) `[]VersionedResource`, instead of instantiating `VersionedResource` ad-hoc.
>
> To resolve that conflict I think I need to implement the suggestion from [#571 (comment)](https://github.com/vmware-tanzu/carvel-kapp/pull/571#discussion_r989791601), extracting `existingVersionedResources` and `newVersionedResources` to a new struct.
>
> [...]
>
> ```go
> // cmd/app/deploy.go
> // func (o *DeployOptions) calculateAndPresentChanges(...)
> //...
> diffResources := ctldiff.NewDiffResourceFactory(existingResource, newResources,
> conf.TemplateRules(), conf.StripNameHashSuffixConfigs()).NewDiffResources()
>
> err := ctldiff.NewRenewableResource(diffResources).Prepare()
> // ...
>
> changes, err := ctldiff.NewChangeSetWithVersionedRs(diffResources,
> opts ChangeSetOps, changeFactory ChangeFactory).Calculate()
> // ...
> ```
>
> with:
>
> ```go
> package diff
>
> type DiffResources struct { // or any other name, only was the first one that came to my mind ^^
> ExisitingResources, NewResources versionedResources
> ExistingResourcesGrouped map[string][]VersionedResource
> // NB: While RenewableResources wants a map[string]ctlres.Resource
> // it should also be able to use [-1]VersionedResource.Res().
> }
>
> type DiffResourcesFactory struct { // or VersionedResourcesFactory?
> // attributes extracted from ChangeSetWithVersionedRs, only used by the extracted methods (only).
> existingRs, newRs []ctlres.Resource
> rules []ctlconf.TemplateRules
> stripNameHashSuffixConfig stripNameHashSuffixConfig
> // extracted attributes end
> }
>
> func NewDiffResourcesFactory(exisitingRs, newRs []ctlres.Resource,
> rules []ctlconf.TemplateRule,
> stripNameHashSuffixConfigs []ctlconf.StripNameHashSuffixConfig) DiffResourcesFactory {
>
> return DiffResourcesFactory{existingRs, newRs,
> rules, stripNameHashSuffixConfigFromConf(stripNameHashSuffixConfigs)}
> }
>
> func (d DiffResourcesFactory) NewDiffResources() DiffResources {
> result := DiffResources{}
> result.ExistingResources = d.existingVersionedRs()
> result.ExistingResourcesGrouped = d.existingVersionedRsGrouped()
> result.NewResources = d.newVersionedRs()
> return result
> }
>
> // methods extracted from ChangeSetWithVersionedRs
>
> func (d DiffResourcesFactory) existingVersionedRs() versionedResources {
> // existingVersionedResources
> }
>
> func (d DiffResourcesFactory) existingVersionedRsGrouped() map[string][]VersionedResource {
> // groupResources / newGroupedVersionedResources
> }
>
> func (d DiffResourcesFactory) newVersionedRs() versionedResources {
> // newVersionedResources
> }
> ```

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.