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

Refactor /inventory code to expose some helpers #4090

Merged
merged 10 commits into from
Nov 8, 2023
Merged

Conversation

foot
Copy link
Contributor

@foot foot commented Oct 18, 2023

weaveworks/weave-gitops-enterprise#2776

What changed?

  • Refactor inventory code
  • Expose some of it so we can use it any parts of the code eg: in wge
  • Fixes logic to look for children in the parentObjects namespace, not the automation's namespace

Why was this change made?

Other similar CRDs that match the flux inventory pattern can share similar code like this

How was this change implemented?

How did you validate the change?
unit tests

Release notes
Expose some inventory directory helper functions

Documentation Changes

@ranatrk ranatrk force-pushed the expose-inventory-helpers branch 3 times, most recently from dbea5b3 to dae5fc6 Compare November 2, 2023 13:17
@ranatrk ranatrk marked this pull request as ready for review November 2, 2023 13:53
@ranatrk ranatrk added the type/enhancement New feature or request label Nov 2, 2023
@ranatrk ranatrk requested a review from a team November 2, 2023 14:19
core/server/inventory.go Outdated Show resolved Hide resolved
result = append(result, entry)
resultMu.Unlock()
}(*o)
// FIXME: do we need this?
Copy link
Contributor

Choose a reason for hiding this comment

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

When you are getting them from Kubernetes, they should have their namespace (even if it's corev1.NamespaceDefault).

if unstructuredObj.GetKind() == "Secret" {
unstructuredObj, err = sanitizeUnstructuredSecret(unstructuredObj)
var err error
unstructuredObj, err = SanitizeUnstructuredSecret(unstructuredObj)
Copy link
Contributor

Choose a reason for hiding this comment

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

SanitizeUnstructuredSecret isn't used outside of the server package?

Copy link
Contributor

Choose a reason for hiding this comment

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

no, i'll make it private

Copy link
Contributor Author

@foot foot left a comment

Choose a reason for hiding this comment

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

LGTM!

@foot foot merged commit 8e11144 into main Nov 8, 2023
17 checks passed
@foot foot deleted the expose-inventory-helpers branch November 8, 2023 12:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants