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

feat(composite): Add support for EnvironmentConfigs #353

Merged
merged 1 commit into from
Sep 7, 2022

Conversation

MisterMX
Copy link
Contributor

@MisterMX MisterMX commented Sep 6, 2022

Description of your changes

Adds interface and mock functions to allow storing references to EnvironmentConfigs in XRs (spec.environmentConfigRefs).

Ref. design document crossplane/crossplane#3008.

I have:

  • Read and followed Crossplane's contribution process.
  • Run make reviewable test to ensure this PR is ready for review.

How has this code been tested

n.A.

Signed-off-by: Maximilian Blatt <maximilian.blatt-extern@deutschebahn.com>
(external expert on behalf of DB Netz AG)
@MisterMX MisterMX requested a review from negz September 6, 2022 13:13
@MisterMX MisterMX changed the title feat(composite): AAdd msupport for EnvironmentConfigs feat(composite): Add msupport for EnvironmentConfigs Sep 6, 2022
@MisterMX MisterMX changed the title feat(composite): Add msupport for EnvironmentConfigs feat(composite): Add support for EnvironmentConfigs Sep 6, 2022
Comment on lines +234 to +235
// TODO(negz): Ask muvaf to explain what this is working around. :)
// TODO(muvaf): temporary workaround.
Copy link
Member

Choose a reason for hiding this comment

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

I think @muvaf is on vacation, but I'd still love to know what this is doing. 😄

Copy link
Member

Choose a reason for hiding this comment

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

I think it was about having empty elements in the array causing problem since the element would be omitted but I'm not 100% sure.

@negz negz merged commit 619edba into crossplane:master Sep 7, 2022
}
filtered = append(filtered, ref)
}
_ = fieldpath.Pave(c.Object).SetValue("spec.resourceRefs", filtered)
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't it set spec.environmentConfigRefs here instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes --> #365

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.

4 participants