Improve how citadel decides which set of namespaces to operate on [Issue #14828] #15503

Monkeyanator · 2019-07-12T22:06:01Z

name: Improve how Istio citadel decides which set of namespaces to operate on [Issue #14828]
about: Removed explicit-opt-in flag and introduced new targeting mechanism, based on labeling namespaces with the namespace of the desired Citadel instance

Relevant issue
Relevant design document
PR and comments from personal fork
Recorded Security WG Meeting Discussion on this

This PR introduces two new labels, ca.istio.io/env, which allows users to designate which namespaces (if any) Citadel should operate on, and ca.istio.io/override, which allows users to force Citadel to target or ignore a namespace (supersedes other activation rules). This PR also introduces a new environment variable, enableNamespacesByDefault, which determines the default behavior when the designated label is not found (i.e. whether or not Citadel instance should create secret for added ServiceAccount).

Retroactive secret creation and deletion are also included in this PR (i.e. enabling a namespace will create secrets in the namespace for existing SAs).

Note: this is a hefty PR, and I'd be fine to chop it into multiple, smaller PRs if that seems more manageable/reviewable (separating retroactive secret creation into a different PR / discussion might make sense). Further note: the configuration options are operator facing and will have to be documented.

[ ] Configuration Infrastructure
[ ] Docs
[ ] Installation
[ ] Networking
[ ] Performance and Scalability
[ ] Policies and Telemetry
[X] Security
[ ] Test and Release
[ ] User Experience
[ ] Developer Infrastrcture

Monkeyanator · 2019-07-12T22:08:57Z

@incfly @elevran @costinm fyi

golangcibot · 2019-07-12T22:11:53Z

security/pkg/k8s/controller/workloadsecret.go

+				sc.deleteSecret(name, namespace)
+				return
+			}
+		} else if err != nil { // in the case we couldn't retrieve namespace, we should proceed with cert refresh


nilness: tautological condition: non-nil != nil (from govet)

golangcibot · 2019-07-12T22:11:53Z

security/pkg/k8s/controller/workloadsecret_test.go

+				ktesting.NewCreateAction(nsSchema, "", createNS("false-to-true", map[string]string{})),
+				ktesting.NewCreateAction(saSchema, "false-to-true", createServiceAccount("test-sa", "false-to-true")),
+				ktesting.NewListAction(saSchema, schema.GroupVersionKind{}, "false-to-true", metav1.ListOptions{}),
+				ktesting.NewCreateAction(secretSchema, "false-to-true", ca.BuildSecret("test-sa", "istio.test-sa", "false-to-true", nil, nil, nil, nil, nil, IstioSecretType)),


line is 163 characters (from lll)

howardjohn · 2019-07-13T02:30:48Z

install/kubernetes/helm/istio/charts/security/values.yaml

@@ -14,6 +14,9 @@ citadelHealthCheck: false
 # 90*24hour = 2160h
 workloadCertTtl: 2160h

+# Citadel default behavior if the citadel.istio.io/managed namespace label not found


ca.istio.io?

elevran · 2019-07-15T11:19:47Z

install/kubernetes/helm/istio/charts/security/values.yaml

@@ -14,6 +14,9 @@ citadelHealthCheck: false
 # 90*24hour = 2160h
 workloadCertTtl: 2160h

+# Citadel default behavior if the citadel.istio.io/managed namespace label not found
+unlabeledNamespacesManaged: true


should we opt-in to Citadel or opt-out by default?
I find opt-in behavior easier to reason as I assume existing namespaces would not be labeled accordingly.

Wouldn't opt-out match the current behavior (i.e. manageUnlabeled=true as the default)? As it is, Citadel will operate on all namespaces it gets updates about (i.e. those specified in labeled-namespaces), labeled or not.

elevran · 2019-07-15T11:21:15Z

security/cmd/istio_ca/main.go

@@ -160,11 +158,10 @@ func init() {
 	flags.StringVar(&opts.listenedNamespaces, "listened-namespaces", metav1.NamespaceAll,
 		"Select the namespaces for the Citadel to listen to, separated by comma. If unspecified, Citadel tries to use the ${"+
 			cmd.ListenedNamespaceKey+"} environment variable. If neither is set, Citadel listens to all namespaces.")
+	flags.BoolVar(&opts.unlabeledNamespacesManaged, "unlabeled-namespaces-managed", true, "Determines whether Citadel"+
+		"should generate secrets for service accounts created in namespaces without the citadel.istio.io/managed label")


ca.istio.io?

elevran · 2019-07-15T11:25:39Z

security/cmd/istio_ca/main.go

-	kubeConfigFile          string
-	readSigningCertOnly     bool
+	listenedNamespaces         string
+	unlabeledNamespacesManaged bool


nit: active var name (e.g., manageUnlabeled) may be a little easier (try both options in an 'if' statement...)

elevran · 2019-07-15T11:30:31Z

security/cmd/istio_ca/main.go

@@ -306,11 +303,15 @@ func runCA() {
 	stopCh := make(chan struct{})
 	if !opts.serverOnly {
 		log.Infof("Creating Kubernetes controller to write issued keys and certs into secret ...")
+
+		citadelNamespace := env.RegisterStringVar("CITADEL_NS", "",


is using CITADEL_ prefix commonly used for other env-variables?
nit: also, since the definition and use are so close (in terms of LoC) a short variable name (ns) may be more idiomatic.

Agreed on ns being more idiomatic, fixed.

Did some digging on what the best prefix for the envvar would be. From here, seems like POD_NAMESPACE would work fine.

elevran · 2019-07-15T11:32:48Z

security/pkg/k8s/controller/workloadsecret.go

+	unlabeledNamespacesManaged bool
+
+	// Used to coordinate with label and check if this instance of Citadel should create secret
+	citadelNamespace string


self namespace? They are all Citadels...

elevran · 2019-07-15T11:41:15Z

security/pkg/k8s/controller/workloadsecret.go

@@ -188,6 +197,20 @@ func NewSecretController(ca ca.CertificateAuthority, requireOptIn bool, certTTL
 			UpdateFunc: c.scrtUpdated,
 		})

+	namespaceLW := listwatch.MultiNamespaceListerWatcher(namespaces, func(namespace string) cache.ListerWatcher {
+		return &cache.ListWatch{
+			ListFunc: func(options metav1.ListOptions) (runtime.Object, error) {


question: is it possible to pass a label selector to the NS watcher to return only relevant namespaces (at least when not managing unlabeled namespaces)?

costinm · 2019-07-15T20:14:00Z

install/kubernetes/helm/istio/charts/security/templates/deployment.yaml

@@ -63,11 +63,17 @@ spec:
          {{- if .Values.workloadCertTtl }}
            - --workload-cert-ttl={{ .Values.workloadCertTtl }}
          {{- end }}
+            - --manage-unlabeled={{ .Values.manageUnlabeled }}


If value is set, so it reduces the upgrade risks ( version in the repo not having the flag )

Right, agreed. I was having a hard time figuring out how to accomplish this, since

{{- if .Values.manageUnlabeled }} - --manage-unlabeled={{ .Values.manageUnlabeled }} {{- end }}

would actually leave the CLI flag out altogether in the case that the user set it to false (since Helm does not distinguish between the value not existing and the value existing as false in this context).

Any ideas?

costinm · 2019-07-15T20:14:53Z

install/kubernetes/helm/istio/charts/security/values.yaml

@@ -14,6 +14,9 @@ citadelHealthCheck: false
 # 90*24hour = 2160h
 workloadCertTtl: 2160h

+# Citadel default behavior if the ca.istio.io/managed namespace label not found


Can we be consistent with what injector uses ?
Also I think we agreed that it won't be a boolean in the label.

this specifies behavior when no label is found, what citadel is supposed to do.

the actual label is not using boolean.

costinm · 2019-07-15T20:16:45Z

security/cmd/istio_ca/main.go

@@ -160,11 +158,10 @@ func init() {
 	flags.StringVar(&opts.listenedNamespaces, "listened-namespaces", metav1.NamespaceAll,
 		"Select the namespaces for the Citadel to listen to, separated by comma. If unspecified, Citadel tries to use the ${"+
 			cmd.ListenedNamespaceKey+"} environment variable. If neither is set, Citadel listens to all namespaces.")
+	flags.BoolVar(&opts.manageUnlabeled, "manage-unlabeled", true, "Determines whether Citadel"+


I think we discussed not adding flags, but instead using a config file that can be updated without restart.
Adding values.yaml or the citadel subsection to a config map would be better than flag, and avoid the complexity of reading config, installer adding flag, restart, etc.

I believe Galley already does this with viper package.

Agreed this would be ideal, but should it perhaps be done in a separate PR? If we decide we want Citadel to support hot reloading config, we would want to move other flags into the config map as well. Might be a task worthy of its own review

security/cmd/istio_ca/main.go

costinm · 2019-07-15T20:19:55Z

security/pkg/k8s/controller/workloadsecret.go

@@ -41,6 +41,10 @@ const (
 	// The Istio secret annotation type
 	IstioSecretType = "istio.io/key-and-cert"

+	// Label which determines whether Citadel should watch namespace
+	NamespaceManagedLabel = "ca.istio.io/secret-controller-namespace"


Labels and annotations should be in pkg/ something - since they may need to be used from multiple packages.
Possibly in api.

Also - please don't use 'secret-controller-namespace' - doesn't make sense from multiple reasons.
SDS is not a secret controller, it may be remote (not in a namespace), etc.

Maybe ca.istio.io/address or ca.istio.io/provider ( this MUST be discussed cross WG, since the pattern should apply to any other component, we can't use a different way for each WG. ).

For secret-controller Can you suggest better name and explains how it fits well with SDS? If there's some doc settled done, will be even better.

As for choosing namespace, we talked with @howardjohn and learned that existing configuration for installer is using namespace, the full url sounds flexible but does not exist yet.

costinm · 2019-07-15T20:20:33Z

security/pkg/k8s/controller/workloadsecret.go

@@ -41,6 +41,10 @@ const (
 	// The Istio secret annotation type
 	IstioSecretType = "istio.io/key-and-cert"

+	// Label which determines whether Citadel should watch namespace
+	NamespaceManagedLabel = "ca.istio.io/secret-controller-namespace"
+	NamespaceIgnoreValue  = "NONE"


What is this and why ? Comments, etc.

If the NamespaceManagedLabel has the value of NamespaceIgnoreValue, all Citadels will ignore the namespace. Note that the Citadels will ignore the namespace if the label has any value that doesn't match their own namespaces, but it makes sense to have a designated one as well.

This extra magic-value for disabling a NS across all Citadels is admittedly a bit clunky, would be open to suggestions.

costinm · 2019-07-15T20:22:05Z

security/pkg/k8s/controller/workloadsecret.go

+	manageUnlabeled bool
+
+	// Used to coordinate with label and check if this instance of Citadel should create secret
+	selfNamespace string


I believe we received feedback on injector that we need to support finer granularity - please use either serviceName or URL to identify the CA that manages a namespace, and match against it.

That would allow a canary CA in same namespace as the prod one.

Reiterating the comment from @incfly here wrt using a URI as the granularity of selection.

As for using serviceName, is there a good mechanism for a Pod to know what Service it's running under? Don't think it's as straightforward to expose this to the pod as it is to expose the namespace it's running under

costinm · 2019-07-15T20:23:39Z

security/pkg/k8s/controller/workloadsecret.go

@@ -188,6 +197,20 @@ func NewSecretController(ca ca.CertificateAuthority, requireOptIn bool, certTTL
 			UpdateFunc: c.scrtUpdated,
 		})

+	namespaceLW := listwatch.MultiNamespaceListerWatcher(namespaces, func(namespace string) cache.ListerWatcher {


Since this pattern of using namespace labels to manage or not is common - maybe the code should be in pkg/ (if it's not already there ?). Would avoid more fragmentation and confusion...
@geeknoid

sounds good to me, but might be done in a separate PR.

costinm · 2019-07-15T20:24:20Z

security/pkg/k8s/controller/workloadsecret.go

@@ -209,40 +233,27 @@ func GetSecretName(saName string) string {

 // Determine if the object is "enabled" for Istio.
 // Currently this looks at the list of watched namespaces and the object's namespace annotation
-func (sc *SecretController) istioEnabledObject(obj metav1.Object) bool {
-	if _, watched := sc.namespaces[obj.GetNamespace()]; watched || !sc.explicitOptIn {
+func (sc *SecretController) istioManagedObject(obj metav1.Object) bool {


Doesn't make sense - an object can be managed by Istio, just not by this particular instance.

Agreed, renamed to citadelManagedObject.

costinm

Implementation looks ok - API is the primary concern, in particular consistency across WG.
Please make sure the actual label and semantics are discussed/approved in TOC or API WG, but it can't be one-off for citadel.

Also I strongly suggest moving the selection logic to common.

costinm · 2019-07-16T04:06:14Z

On Mon, Jul 15, 2019 at 3:19 PM Sam Naser ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In security/pkg/k8s/controller/workloadsecret.go <#15503 (comment)>: > @@ -41,6 +41,10 @@ const ( // The Istio secret annotation type IstioSecretType = "istio.io/key-and-cert" + // Label which determines whether Citadel should watch namespace + NamespaceManagedLabel = "ca.istio.io/secret-controller-namespace" + NamespaceIgnoreValue = "NONE" If the NamespaceManagedLabel has the value of NamespaceIgnoreValue, all Citadels will ignore the namespace. Note that the Citadels will ignore the namespace if the label has *any* value that doesn't match its namespace, but it makes sense to have a designated one as well. This extra magic-value for disabling a NS across all Citadels is admittedly a bit clunky, would be open to suggestions.

Again - let's try to be consistent. Check the injector - and the new model used by istio/installer, where the label on namespace selects which injector to use. I believe the is a injection-disabled=true label - don't remember now. I'm not sure I agree we need all the combinations - Istio is complex enough already. The use cases are to use a different CA on specific namespaces while default citadel generates for namespaces without a label. Or to not have a default at all.

…

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#15503?email_source=notifications&email_token=AAAUR2SRZT3CPYYLXI3OLCTP7TZVXA5CNFSM4ICSFA32YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOB6PWLDA#discussion_r303660531>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAAUR2RVNNXWYQ2NV2UW4HDP7TZVXANCNFSM4ICSFA3Q> .

costinm · 2019-07-16T04:12:39Z

On Mon, Jul 15, 2019 at 1:34 PM Jianfei Hu ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In security/pkg/k8s/controller/workloadsecret.go <#15503 (comment)>: > @@ -41,6 +41,10 @@ const ( // The Istio secret annotation type IstioSecretType = "istio.io/key-and-cert" + // Label which determines whether Citadel should watch namespace + NamespaceManagedLabel = "ca.istio.io/secret-controller-namespace" For secret-controller Can you suggest better name and explains how it fits well with SDS? If there's some doc settled done, will be even better. As for choosing namespace, we talked with @howardjohn <https://github.com/howardjohn> and learned that existing configuration for installer is using namespace, the full url sounds flexible but does not exist yet.

- ca.istio.io/env ( consistent with istio-env used by injector - we may change it but then at least we change all consistently ) - ca.istio.io/profile ? - ca.istio.io/address ? Not easy to find a good name - but keep in mind we'll be stuck with it, very hard to change it later. Secret-controler is bad because we want to use this to select an SDS environment as well. namespace - while new installer is using namespaces, the feedback we got is to not do that, but allow different 'environments' or versions to be in the same namespaces. Maybe ca.istio.io/instance or /id or ca.istio.io/name ?

…

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#15503?email_source=notifications&email_token=AAAUR2VJTBZD43MMXCCRQY3P7TNONA5CNFSM4ICSFA32YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOB6PLLHA#discussion_r303623713>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAAUR2U5YZPC5WMS5MLCJHTP7TNONANCNFSM4ICSFA3Q> .

costinm · 2019-07-16T04:14:02Z

Even if it is possible - this is already far more complicated than it should be, in particular for something we may deprecate after SDS is stable.

…

On Mon, Jul 15, 2019 at 4:41 AM Etai Lev Ran ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In security/pkg/k8s/controller/workloadsecret.go <#15503 (comment)>: > @@ -188,6 +197,20 @@ func NewSecretController(ca ca.CertificateAuthority, requireOptIn bool, certTTL UpdateFunc: c.scrtUpdated, }) + namespaceLW := listwatch.MultiNamespaceListerWatcher(namespaces, func(namespace string) cache.ListerWatcher { + return &cache.ListWatch{ + ListFunc: func(options metav1.ListOptions) (runtime.Object, error) { question: is it possible to pass a label selector to the NS watcher to return only relevant namespaces (at least when not managing unlabeled namespaces)? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#15503?email_source=notifications&email_token=AAAUR2XZ5G22KNG464GRBCLP7RO55A5CNFSM4ICSFA32YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOB6NDZFA#pullrequestreview-261766292>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAAUR2TQLADF67WGSY5CUFTP7RO55ANCNFSM4ICSFA3Q> .

Monkeyanator · 2019-07-16T17:32:44Z

There definitely needs to be consistency between this configuration story and the sidecar injector's. Quoting from https://github.com/istio/installer#auto-injection, the documented behavior for the sidecar injector is:

If installed, namespaces can select the injector by setting the istio-env label on the namespace.

This corresponds to the ca.istio.io/secret-controller-namespace label, which we can rename to ca.istio.io/env or something more appropriate as discussed. As it stands, this points to a namespace, both in the sidecar injector from the installer and here, and when the time comes to use a better specifier (e.g. URI), we should migrate them together.

Only one auto-injector environment should have enableNamespacesByDefault=true, which will apply that environment to any namespace without an explicit istio-env label.

This corresponds to the manageUnlabeled CLI flag from this PR, which describes what to do with unlabeled namespaces. We could rename this to match enableNamespacesByDefault for consistency.

If istio-system has set enableNamespaceByDefault you must set istio-inject: disabled label to prevent istio-system from taking over. In this case, it is recommended to first install istio-control autoinject with the default disabled, test it, and move the default from istio-system to istio-control.

This aligns with the NamespaceIgnoreValue = "NONE" special value proposed in this PR. The alternative would be to do what the sidecar injector does and add another special override label, something like ca.istio.io:{enabled/disabled}.

I think this covers all the inconsistencies between the two configuration stories, let me know if I'm missing anything. (to the point of the complexity: the interactions between the labels and defaults is definitely tedious, but the truth table for the sidecar injector follows the same behavior)

Monkeyanator · 2019-07-30T00:45:02Z

@costinm fixed the name of the option (back to enableNamespacesByDefault) 👍 as for the prefix change, I think that might take some additional thought/design- since this PR is already large enough, I say we make a separate PR for that work.

costinm · 2019-07-30T15:51:26Z

Citadel has a number of options that don't make sense from installer / operations perspective - not clear why they were added, but the storage namespace is one of them. As we simplify and cleanup we should fix some of the arbitrary configurability. On your previous comment - VERSION will be something like 'canary' or 'prod-sds' - not 1.3.14.

…

On Mon, Jul 29, 2019 at 9:41 AM Sam Naser ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In security/cmd/istio_ca/main.go <#15503 (comment)>: > @@ -262,6 +263,16 @@ func init() { cmd.InitializeFlags(rootCmd) } +func initEnvVars() { + selfNamespace := env.RegisterStringVar("POD_NAMESPACE", "", Looked a bit more into this -- from the comment on the CLI flag, istioCaStorageNamespace is the "namespace where the Citadel pod is running. Will not be used if explicit file or other storage mechanism is specified". Just to make sure: this will always be set to the namespace where the pod is running in, even if a different storage mechanism is used, correct? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#15503?email_source=notifications&email_token=AAAUR2S5SLDIX25E7Z3SWZDQB4MR5A5CNFSM4ICSFA32YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOB736EJA#discussion_r308327559>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAAUR2WRCYD6LEQ46BW43CDQB4MR5ANCNFSM4ICSFA3Q> .

Monkeyanator · 2019-07-30T16:11:55Z

/test istio-unit-tests-master

Monkeyanator · 2019-07-30T18:00:53Z

/test istio-unit-tests-master

Monkeyanator · 2019-07-31T17:04:21Z

/test istio-unit-tests-master

incfly

/lgtm
/approve

some nitpicks on comments, otherwise good to go.
also feel free to address as a separate PR though.

incfly · 2019-07-31T20:02:06Z

install/kubernetes/helm/istio/charts/security/values.yaml

@@ -14,6 +14,16 @@ citadelHealthCheck: false
 # 90*24hour = 2160h
 workloadCertTtl: 2160h

+# Determines Citadel default behavior if the ca.istio.io/env or ca.istio.io/override
+# namespace are not found on a given namespace.


s/namespace/label?

xxx and yyy label are not found a given namespac.e

incfly · 2019-07-31T20:04:37Z

security/pkg/k8s/controller/workloadsecret.go

+	// whether or not a given Citadel instance should operate on a namespace. The behavior is as follows:
+	// 1) If NamespaceOverrideLabel exists and is valid, follow what this label tells us
+	// 2) If not, check NamespaceManagedLabel. If the value matches the Citadel instance's NS, then it should be active
+	// 3) If NamespaceManagedLabel nonexistent or invalid, follow enableNamespacesByDefault environment variable


follow enableNamespaceByDefault, controlled by CITALDE_XXX environment variable.

the first is the field name, the second us user facing config, the actual env var name.

incfly · 2019-07-31T20:06:10Z

security/pkg/k8s/controller/workloadsecret.go

 	ns, err := sc.core.Namespaces().Get(obj.GetNamespace(), metav1.GetOptions{})
-	if err != nil || ns == nil { // @todo handle errors? Unit tests mocks don't create NS, only secrets
-		return enabled
+	if err != nil || ns == nil { // if we can't retrieve namespace details, fall back on default value


AFAIK, before this PR we don't check namespace label details, thus always 100% to proceed with handling secrets/key generation.

Fallback to default behavior seems align with existing behavior better.

Monkeyanator · 2019-07-31T20:23:28Z

@incfly since they're so minor I'll go ahead and fix the nits now

/hold

Introduce new designated labels ca.istio.io/env and ca.istio.io/override. Check for presence of these options before creating secrets for a ServiceAccount, and ensure the Citadel instance is in the namespace designated in the label value or has override label set. The enableNamespacesByDefault option determines the default behavior in the case that these labels are not found. Added logic for retroactive creation and deletion of secrets in activated / deactivated namespaces.

Monkeyanator · 2019-07-31T20:34:00Z

/hold cancel

incfly · 2019-07-31T20:39:02Z

/lgtm
/approve

istio-testing · 2019-07-31T20:39:16Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: costinm, incfly, Monkeyanator

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:

~~install/OWNERS~~ [costinm]
~~security/OWNERS~~ [costinm,incfly]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Monkeyanator · 2019-07-31T21:35:58Z

/test istio-pilot-e2e-envoyv2-v1alpha3-master

istio-testing · 2019-08-01T00:36:31Z

@Monkeyanator: The following tests failed, say /retest to rerun them all:

Test name	Commit	Details	Rerun command
prow/istio-lint.sh	c25d9fc788830fa42e17ce5d8acfe0958b709490	link	`/test istio-lint-master`
prow/e2e-bookInfoTests-v1alpha3.sh	c25d9fc788830fa42e17ce5d8acfe0958b709490	link	`/test e2e-bookInfoTests-envoyv2-v1alpha3-master`
prow/integ-mixer-k8s-presubmit-tests.sh	c25d9fc788830fa42e17ce5d8acfe0958b709490	link	`/test integ-mixer-k8s-presubmit-tests-master`
prow/integ-new-installer-k8s-presubmit-tests.sh	c25d9fc788830fa42e17ce5d8acfe0958b709490	link	`/test integ-new-install-k8s-presubmit-tests-master`
integ-new-install-k8s-presubmit-tests-master	`bc0b1dc`	link	`/test integ-new-install-k8s-presubmit-tests-master`

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

Monkeyanator · 2019-08-01T02:57:56Z

/test istio-racetest-master

* Sync changes to old installer Cherrypicks istio#13392, istio#13666, istio#13982, istio#14077, istio#14059 * Cherrypick istio#14301 * Cherrypick istio#13638 * cherrypick istio#14606 * istio#14171 * istio#14438 * istio#14674 * istio#14974 * 14904 * istio#14796 * istio#15346 * istio#15345 * istio#15383 * istio#14815 * istio#15690 * istio#15681 * istio#15014 * istio#15503 * istio#16084 * istio#16146 * istio#16147 * Fix tests

istio-testing added the area/security label Jul 12, 2019

googlebot added the cla: yes label Jul 12, 2019

istio-testing requested review from myidpt and ostromart July 12, 2019 22:06

golangcibot reviewed Jul 12, 2019

View reviewed changes

howardjohn reviewed Jul 13, 2019

View reviewed changes

elevran reviewed Jul 15, 2019

View reviewed changes

costinm reviewed Jul 15, 2019

View reviewed changes

security/cmd/istio_ca/main.go Show resolved Hide resolved

costinm reviewed Jul 15, 2019

View reviewed changes

istio-testing added needs-rebase Indicates a PR needs to be rebased before being merged size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed needs-rebase Indicates a PR needs to be rebased before being merged labels Jul 17, 2019

istio-testing added the approved label Jul 29, 2019

geeknoid added cla: yes Set by the Google CLA bot to indicate the author of a PR has signed the Google CLA. and removed cla: yes_old labels Jul 30, 2019

istio-testing removed the lgtm label Jul 30, 2019

sushicw mentioned this pull request Jul 30, 2019

Add EnableWebhookNamespaceSelector flag to galley for webhook config #15927

Closed

incfly approved these changes Jul 31, 2019

View reviewed changes

istio-testing added the lgtm label Jul 31, 2019

istio-testing removed the lgtm label Jul 31, 2019

istio-testing added the lgtm label Jul 31, 2019

Monkeyanator mentioned this pull request Aug 1, 2019

Add docs for updated Citadel namespace targeting mechanism istio/istio.io#4746

Merged

istio-testing merged commit 3ef5a60 into istio:master Aug 1, 2019

This was referenced Aug 4, 2019

Citadel requires k8s namespace read permissions but does not have them #15115

Closed

Generate Istio certs on demand #13836

Closed

Monkeyanator mentioned this pull request Aug 7, 2019

Citadel namespace targeting integration test #16090

Merged

Monkeyanator mentioned this pull request Aug 16, 2019

Fix Citadel clusterrole and tracing image istio/installer#335

Merged

howardjohn mentioned this pull request Oct 1, 2019

Citadel can no longer run in a single namespace in 1.3.0 due to listing of namespaces #17309

Closed

Improve how citadel decides which set of namespaces to operate on [Issue #14828] #15503

Improve how citadel decides which set of namespaces to operate on [Issue #14828] #15503

Conversation

Monkeyanator commented Jul 12, 2019 • edited

name: Improve how Istio citadel decides which set of namespaces to operate on [Issue #14828] about: Removed explicit-opt-in flag and introduced new targeting mechanism, based on labeling namespaces with the namespace of the desired Citadel instance

Monkeyanator commented Jul 12, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Monkeyanator Jul 15, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elevran Jul 15, 2019 • edited

Choose a reason for hiding this comment

Monkeyanator Jul 15, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Monkeyanator Jul 15, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Monkeyanator Jul 15, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Monkeyanator Jul 15, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

costinm left a comment

Choose a reason for hiding this comment

costinm commented Jul 16, 2019 via email

costinm commented Jul 16, 2019 via email

costinm commented Jul 16, 2019 via email

Monkeyanator commented Jul 16, 2019

Monkeyanator commented Jul 30, 2019 • edited

costinm commented Jul 30, 2019 via email

Monkeyanator commented Jul 30, 2019

Monkeyanator commented Jul 30, 2019

Monkeyanator commented Jul 31, 2019

incfly left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Monkeyanator commented Jul 31, 2019

Monkeyanator commented Jul 31, 2019

incfly commented Jul 31, 2019

istio-testing commented Jul 31, 2019

Monkeyanator commented Jul 31, 2019

istio-testing commented Aug 1, 2019 • edited

Monkeyanator commented Aug 1, 2019

Monkeyanator commented Jul 12, 2019 •

edited

name: Improve how Istio citadel decides which set of namespaces to operate on [Issue #14828]
about: Removed explicit-opt-in flag and introduced new targeting mechanism, based on labeling namespaces with the namespace of the desired Citadel instance

Monkeyanator Jul 15, 2019 •

edited

elevran Jul 15, 2019 •

edited

Monkeyanator Jul 15, 2019 •

edited

Monkeyanator Jul 15, 2019 •

edited

Monkeyanator Jul 15, 2019 •

edited

Monkeyanator Jul 15, 2019 •

edited

Monkeyanator commented Jul 30, 2019 •

edited

istio-testing commented Aug 1, 2019 •

edited