Skip to content

Conversation

@oseoin
Copy link
Contributor

@oseoin oseoin commented Dec 6, 2023

Proposed changes

  • Refactor the following functions to use config structs instead of excessive parameters
    • generateNginxCfg()
    • generateNginxCfgForMergeableIngresses()
    • NewConfigurator()
  • Move repeated service binary arguments in Helm chart from deployment and daemonset to _helpers

Checklist

Before creating a PR, run through this checklist and mark each as complete.

  • I have read the CONTRIBUTING doc
  • I have added tests that prove my fix is effective or that my feature works
  • I have checked that all unit tests pass after adding my changes
  • I have updated necessary documentation
  • I have rebased my branch onto main
  • I will ensure my PR is targeting the main branch and pulling from my branch from my own fork

@oseoin oseoin requested a review from a team as a code owner December 6, 2023 16:57
@github-actions github-actions bot added the helm_chart Pull requests that update the Helm Chart label Dec 6, 2023
…nc/kubernetes-ingress into helm-args-and-config-sig-refactor
Copy link
Contributor

@jjngx jjngx left a comment

Choose a reason for hiding this comment

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

👍🏻

@codecov
Copy link

codecov bot commented Dec 7, 2023

Codecov Report

Attention: 34 lines in your changes are missing coverage. Please review.

Comparison is base (12045f5) 52.09% compared to head (95cec66) 52.14%.

Files Patch % Lines
internal/configs/ingress.go 77.89% 16 Missing and 5 partials ⚠️
cmd/nginx-ingress/main.go 0.00% 13 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4758      +/-   ##
==========================================
+ Coverage   52.09%   52.14%   +0.05%     
==========================================
  Files          59       59              
  Lines       17033    17083      +50     
==========================================
+ Hits         8873     8908      +35     
- Misses       7862     7875      +13     
- Partials      298      300       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@oseoin oseoin merged commit e89b329 into main Dec 7, 2023
@oseoin oseoin deleted the helm-args-and-config-sig-refactor branch December 7, 2023 11:21
pdabelf5 pushed a commit that referenced this pull request Dec 7, 2023
* refactor helm args for deploy and daemonset, refactor several function signatures
pdabelf5 added a commit that referenced this pull request Dec 8, 2023
* Bump the go group with 3 updates

Bumps the go group with 3 updates: [github.com/aws/aws-sdk-go-v2/config](https://github.com/aws/aws-sdk-go-v2), [github.com/aws/aws-sdk-go-v2/service/marketplacemetering](https://github.com/aws/aws-sdk-go-v2) and [github.com/nginxinc/nginx-prometheus-exporter](https://github.com/nginxinc/nginx-prometheus-exporter).


Updates `github.com/aws/aws-sdk-go-v2/config` from 1.25.10 to 1.25.12
- [Release notes](https://github.com/aws/aws-sdk-go-v2/releases)
- [Commits](aws/aws-sdk-go-v2@config/v1.25.10...config/v1.25.12)

Updates `github.com/aws/aws-sdk-go-v2/service/marketplacemetering` from 1.19.1 to 1.19.3
- [Release notes](https://github.com/aws/aws-sdk-go-v2/releases)
- [Commits](aws/aws-sdk-go-v2@v1.19.1...service/mq/v1.19.3)

---
updated-dependencies:
- dependency-name: github.com/aws/aws-sdk-go-v2/config
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: go
- dependency-name: github.com/aws/aws-sdk-go-v2/service/marketplacemetering
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: go
...

Signed-off-by: dependabot[bot] <[email protected]>

* Helm and function signature refactors (#4758)

* refactor helm args for deploy and daemonset, refactor several function signatures

* update helm k8s schema validation to 1.28.0 (#4763)


---------

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: oseoin <[email protected]>
Co-authored-by: Paul Abel <[email protected]>
Co-authored-by: Paul Abel <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

helm_chart Pull requests that update the Helm Chart

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants