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: Create endpoint for fetching the flux namespace #1515

Merged
merged 2 commits into from Feb 25, 2022

Conversation

yitsushi
Copy link
Contributor

Add new endpoint to query flux namespace. For simplicity, no arguments
on the request and returns only the name of the namespace. That fulfills
the original request so we can remove the constant1 from the UI.

== Considered solutions

  1. Create a general endpoint

The consumer could query namespaces and filter them with provided
filters.

Dropped this solution, because it could open up some weird surfaces
where the client can query arbitrary filters.

  1. Create a general endpoint with strict filtering options

The consumer could query namespaces with a set of filter options and if
they are not specified, throw back an error.

Dropped this solution, because changing the filtering rules would
require code change. The final solution in this change-set has the same
issue, but less complex. If we don't get extra value out of the
complexity, it's not worth it.

== Follow-up

If/when we need a complex endpoint to query all namespaces with filter
options, then we can add one with a plan like:

  • Who/what will use it?
  • What is the desired input and output?
  • How to achieve this?
  • What limitations do we want to implement?
  • How much freedom do we want to give to the consumer?

== Other changes

  • make proto updated a lot of auto-generated files.

  • In tools/dependencies.toml, removed the system-wide installation of
    a tool, because the last thing I want from a project to download a
    binary and place it somewhere with root permission. I'm not giving
    root permissions just to install dependencies.

  • Use tools/bin/tilt in Makefile (see previous entry).

  • Replaced the logic behind getMatchingLabels. Using the with option
    pattern, we can build up more complex labeling queries, and the
    codebase will remain clean.

    match := matchLabel(
      withPartOfLabel(appName),
      withInstanceLabel(appInstance),
      withManagedByLabel(appManager),
    )

Closes #1465

Footnotes

  1. https://github.com/weaveworks/weave-gitops/pull/1451/files#diff-35acf2adc3a7b1021fb41d3473b47e030d3708ba2e8ff7a4b76c13ee80b4076cR42

@yitsushi
Copy link
Contributor Author

Created a new PR, because the previous one was created from my fork and my fork does not have some secrets for PR checkers.

Copy link
Contributor

@jpellizzari jpellizzari left a comment

Choose a reason for hiding this comment

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

Assuming it is the same, LGTM 👍

Nice work on the PR description. Very informative.

This PR will probably cause some conflicts: #1489

It moves things to a fluxruntime.go file.

Add new endpoint to query flux namespace. For simplicity, no arguments
on the request and returns only the name of the namespace. That fulfills
the original request so we can remove the constant[^1] from the UI.

== Considered solutions

1. Create a general endpoint

The consumer could query namespaces and filter them with provided
filters.

Dropped this solution, because it could open up some weird surfaces
where the client can query arbitrary filters.

2. Create a general endpoint with strict filtering options

The consumer could query namespaces with a set of filter options and if
they are not specified, throw back an error.

Dropped this solution, because changing the filtering rules would
require code change. The final solution in this change-set has the same
issue, but less complex. If we don't get extra value out of the
complexity, it's not worth it.

== Follow-up

If/when we need a complex endpoint to query all namespaces with filter
options, then we can add one with a plan like:

* Who/what will use it?
* What is the desired input and output?
* How to achieve this?
* What limitations do we want to implement?
* How much freedom do we want to give to the consumer?

== Other changes

* `make proto` updated a lot of auto-generated files.
* In `tools/dependencies.toml`, removed the system-wide installation of
  a tool, because the last thing I want from a project to download a
  binary and place it somewhere with root permission. I'm not giving
  root permissions just to install dependencies.
* Use `tools/bin/tilt` in `Makefile` (see previous entry).
* Replaced the logic behind `getMatchingLabels`. Using the `with option`
  pattern, we can build up more complex labeling queries, and the
  codebase will remain clean.

```go
match := matchLabel(
  withPartOfLabel(appName),
  withInstanceLabel(appInstance),
  withManagedByLabel(appManager),
)
```

[^1]: https://github.com/weaveworks/weave-gitops/pull/1451/files#diff-35acf2adc3a7b1021fb41d3473b47e030d3708ba2e8ff7a4b76c13ee80b4076cR42

Fixes #1465
@yitsushi yitsushi force-pushed the 1465-get-flux-namespace-endpoint branch from fbf7277 to 69f8960 Compare February 25, 2022 10:56
@yitsushi yitsushi merged commit 576f0a4 into v2 Feb 25, 2022
@ozamosi ozamosi deleted the 1465-get-flux-namespace-endpoint branch May 12, 2022 17:42
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.

None yet

2 participants