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

[bitnami/external-dns] Add RBAC to support F5 VirtualServer external-dns source #15804

Merged
merged 2 commits into from
Apr 3, 2023
Merged

[bitnami/external-dns] Add RBAC to support F5 VirtualServer external-dns source #15804

merged 2 commits into from
Apr 3, 2023

Conversation

mikejoh
Copy link
Contributor

@mikejoh mikejoh commented Mar 30, 2023

Description of the change

With these recently merged PRs in upstream:
kubernetes-sigs/external-dns#3503 - RBAC added upstream in external-dns helm chart.
kubernetes-sigs/external-dns#3162 - code changes in external-dns.

we now need to add the same RBAC to this chart to be able to configure external-dns to list F5 VirtualServer objects in the cluster.

Benefits

We'll be able to configure the --source=f5-virtualserver flag which instructs external-dns to list all VirtualServer objects in the cluster, these objects will be used as a source to create DNS records in the given provider.

Possible drawbacks

Applicable issues

Additional information

Checklist

  • Chart version bumped in Chart.yaml according to semver. This is not necessary when the changes only affect README.md files.
  • Variables are documented in the values.yaml and added to the README.md using readme-generator-for-helm
  • Title of the pull request follows this pattern [bitnami/<name_of_the_chart>] Descriptive title
  • All commits signed off and in agreement of Developer Certificate of Origin (DCO)

@mikejoh
Copy link
Contributor Author

mikejoh commented Apr 3, 2023

@javsalgar 👋 would it be possible to review this one? Thanks!

Signed-off-by: Mikael Johansson <mik.json@gmail.com>
@javsalgar javsalgar added the verify Execute verification workflow for these changes label Apr 3, 2023
@github-actions github-actions bot added in-progress and removed triage Triage is needed labels Apr 3, 2023
@bitnami-bot bitnami-bot removed the request for review from javsalgar April 3, 2023 07:54
Copy link
Contributor

@aoterolorenzo aoterolorenzo left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution @mikejoh !

I just left you a minor comment above.

@@ -24,4 +24,4 @@ sources:
- https://github.com/kubernetes-sigs/external-dns
- https://github.com/bitnami/containers/tree/main/bitnami/external-dns
- https://github.com/kubernetes-sigs/external-dns
version: 6.15.0
version: 6.15.1
Copy link
Contributor

Choose a reason for hiding this comment

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

Adding functionality should imply a minor bump (Semver summary)

Suggested change
version: 6.15.1
version: 6.16.0

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, of course, fixed here: c4d3d45

Signed-off-by: Mikael Johansson <mik.json@gmail.com>
Copy link
Contributor

@aoterolorenzo aoterolorenzo left a comment

Choose a reason for hiding this comment

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

LGTM

@aoterolorenzo aoterolorenzo merged commit 7f55815 into bitnami:main Apr 3, 2023
@mikejoh mikejoh deleted the external-dns-add-rbac-for-f5-virtualserver branch April 3, 2023 14:02
@mikejoh
Copy link
Contributor Author

mikejoh commented Apr 3, 2023

@aoterolorenzo Sorry for pinging in the PR but i just saw that the pipeline triggered on merge to main failed: https://github.com/bitnami/charts/actions/runs/4597349311/jobs/8119923921. It's due to:

Error: Detected changes in charts without version bump in Chart.yaml. Charts changed: 1 bitnami/external-dns. Version bumps detected: 0

Is that expected? 🤔 Let me know if there's something i can do!

@github-actions github-actions bot removed the solved label Apr 3, 2023
@github-actions github-actions bot added the triage Triage is needed label Apr 3, 2023
Mauraza pushed a commit that referenced this pull request Apr 4, 2023
…dns source (#15804)

* Add RBAC to support F5 VirtualServer external-dns source

Signed-off-by: Mikael Johansson <mik.json@gmail.com>

* Bump minor version

Signed-off-by: Mikael Johansson <mik.json@gmail.com>

---------

Signed-off-by: Mikael Johansson <mik.json@gmail.com>
@github-actions github-actions bot added in-progress and removed triage Triage is needed labels Apr 10, 2023
@carrodher carrodher removed the request for review from dgomezleon April 10, 2023 07:35
@aoterolorenzo
Copy link
Contributor

@aoterolorenzo Sorry for pinging in the PR but i just saw that the pipeline triggered on merge to main failed: https://github.com/bitnami/charts/actions/runs/4597349311/jobs/8119923921. It's due to:

Error: Detected changes in charts without version bump in Chart.yaml. Charts changed: 1 bitnami/external-dns. Version bumps detected: 0

Is that expected? 🤔 Let me know if there's something i can do!

Hi @mikejoh, seems there was a collision when merging two PR's with same version near each other on time. I saw you just bump a minor in #15968 and this indeed have triggered a chart release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
external-dns solved verify Execute verification workflow for these changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants