Configuration should not use env var but `config-kourier`

knative-extensions / net-kourier

Purpose-built Knative Ingress implementation using just Envoy with no additional CRDs

Apache License 2.0

295 stars 82 forks source link

Configuration should not use env var but `config-kourier` #877

Open nak3 opened 2 years ago

nak3 commented 2 years ago

Currently some configurations especially external authorization use env variables as https://github.com/knative-sandbox/net-kourier#external-authorization-configuration instead of config-kourier configmap.

I think that there is no big reason for using env variable but we just used it because we haven't introduced config-kourier at that time.

Now, since new configurations are added in config-kourier, the env variables should be migrated into the configmap.

github-actions[bot] commented 1 year ago

This issue is stale because it has been open for 90 days with no activity. It will automatically close after 30 more days of inactivity. Reopen the issue with /reopen. Mark the issue as fresh by adding the comment /remove-lifecycle stale.

knative-prow-robot commented 1 year ago

This issue or pull request is stale because it has been open for 90 days with no activity.

This bot triages issues and PRs according to the following rules:

After 90d of inactivity, lifecycle/stale is applied
After 30d of inactivity since lifecycle/stale was applied, the issue is closed

You can:

Mark this issue or PR as fresh with /remove-lifecycle rotten
Close this issue or PR with /close

/lifecycle stale

dprotaso commented 1 year ago

/lifecycle frozen

ReToCode commented 1 year ago

/good-first-issue

ReToCode commented 1 year ago

The same would probably apply for CERTS_SECRET_NAMESPACE and CERTS_SECRET_NAME used in caches.go

harshil1973 commented 1 year ago

I would like to contribute in this but I am an absolute beginner if anyone can help me by providing steps it would be nice

nak3 commented 1 year ago

Hmm... I think it is not a "good-first-issue" issue actually. It needs many steps for the change and one big concern is that some configurations might need to be loaded before ConfigMap informer's reconcile loop. How do you think @ReToCode

ReToCode commented 1 year ago

Yeah, I think it could be more complicated than initially though. I haven't checked if any of the configs actually is affected by what you described. But it is probably better to start with a simpler issue @harshil1973.

/remove-good-first-issue