Initial refactors to bootstrap self-contained translator #1101
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: rramkumar1 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
4cef680 to
ccebf6e
Compare
| limitations under the License. | ||
| */ | ||
|
|
||
| package translator |
There was a problem hiding this comment.
There is a translator package under pkg/controller
Do we want to use a single package for all the translator methods?
There was a problem hiding this comment.
This is eventually where I want to get to, yes. I am slowly going to move more and more stuff from pkg/controller/translator to pkg/translator.
ccebf6e to
a8e6007
Compare
227efb7 to
86f2835
Compare
| } | ||
|
|
||
| // TLSCertsFromSecretsLoader loads TLS certs from kubernetes secrets. | ||
| type TLSCertsFromSecretsLoader struct { | ||
| noOPValidator | ||
| Client kubernetes.Interface | ||
| } | ||
|
|
||
| // Ensure that TLSCertsFromSecretsLoader implements TlsLoader interface. | ||
| var _ TlsLoader = &TLSCertsFromSecretsLoader{} | ||
|
|
||
| func (t *TLSCertsFromSecretsLoader) Load(ing *v1beta1.Ingress) ([]*loadbalancers.TLSCerts, error) { |
There was a problem hiding this comment.
Note: this code does not have unit tests right now. I plan on adding some in a follow up PR.
86f2835 to
7dff98a
Compare
7dff98a to
fb59808
Compare
fb59808 to
34f0947
Compare
| // NewEnv returns an Env for the given Ingress. | ||
| func NewEnv(ing *v1beta1.Ingress, client kubernetes.Interface) (*Env, error) { | ||
| ret := &Env{Ing: ing, SecretsMap: make(map[string]*api_v1.Secret)} | ||
| secrets, err := client.CoreV1().Secrets(ing.Namespace).List(context.TODO(), meta_v1.ListOptions{}) |
There was a problem hiding this comment.
this changes from requiring get access only to the secrets referenced in the ingress to list access on all secrets in the namespace. this PR claimed to just be a refactor, I would not expect it to change secret access to list and require expanded permissions.
There was a problem hiding this comment.
There was a problem hiding this comment.
opened #1188 to revert to previous behavior
This is simply moving code around for now. As this translator package evolves, it will become much more self-contained and there will only be a single interaction point (e.g ToCompositeUrlMap will be no longer exported).
This PR has two commits which implements the simplest refactors.
/assign @bowei
This change is