Skip to content
This repository was archived by the owner on May 24, 2023. It is now read-only.

Add tls secret option #131

Merged
merged 1 commit into from
Jul 7, 2021
Merged

Add tls secret option #131

merged 1 commit into from
Jul 7, 2021

Conversation

soneillf5
Copy link
Contributor

@soneillf5 soneillf5 commented Jul 6, 2021

Proposed changes

Describe the use case and detail of the change. If this PR addresses an issue on GitHub, make sure to include a link to that issue here in this description (not in the title of the PR).

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 master
  • I will ensure my PR is targeting the master branch and pulling from my branch from my own fork

@soneillf5 soneillf5 requested review from ciarams87 and vepatel July 6, 2021 15:08
@github-actions github-actions bot added documentation Pull requests/issues for documentation enhancement Pull requests for new features/feature enhancements labels Jul 6, 2021
@@ -147,6 +147,7 @@ spec:
| --- | --- | --- | --- |
| `enable` | `boolean` | Enable Prometheus metrics. | Yes |
| `port` | `int` | Sets the port where the Prometheus metrics are exposed. Default is 9113. Format is `1023 - 65535`. | No |
| `secret` | `string` | Sets the namespace/name of a TLS Secret Resource to use to enable TLS for the Prometheus endpoint. | No |
Copy link

Choose a reason for hiding this comment

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

think need to add arg above line 76 too

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've updated the example if that's what you mean? I don't see anywhere else to add it

Copy link

Choose a reason for hiding this comment

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

yep that was it

@@ -197,6 +197,13 @@ spec:
minimum: 1023
nullable: true
type: integer
secret:
description: A Secret with a TLS certificate and key for TLS termination
Copy link
Contributor

Choose a reason for hiding this comment

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

This description (and the one in bundle) doesn't match the comment in types. I think you need to run make bundle again.

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 I didn't realise it was generated. I ran make bundle and got the following error:
make bundle

/Users/soneill/go/src/github.com/nginxinc/nginx-ingress-operator/bin/controller-gen "crd:trivialVersions=true,preserveUnknownFields=false" rbac:roleName=manager-role webhook paths="./..." output:crd:artifacts:config=config/crd/bases
operator-sdk generate kustomize manifests -q
FATA[0000] failed to read config: error unmarshalling project configuration: error unmarshaling JSON: while decoding JSON: json: cannot unmarshal array into Go struct field Config.layout of type string
make: *** [bundle] Error 1

I've update this PR anyway with he generated changes.

Copy link
Contributor

Choose a reason for hiding this comment

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

The files in bundle are auto-generated too.

@soneillf5 soneillf5 requested review from vepatel and lucacome July 7, 2021 08:53
@soneillf5 soneillf5 force-pushed the feature/prometheus-https branch from f22ac6b to 574a2e6 Compare July 7, 2021 11:52
@soneillf5 soneillf5 dismissed lucacome’s stale review July 7, 2021 11:53

Approved by other reviewers, merge is blocked with timezone difference

@soneillf5 soneillf5 enabled auto-merge (rebase) July 7, 2021 11:54
@soneillf5 soneillf5 merged commit fba21a8 into master Jul 7, 2021
@soneillf5 soneillf5 deleted the feature/prometheus-https branch July 7, 2021 11:54
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
documentation Pull requests/issues for documentation enhancement Pull requests for new features/feature enhancements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants