Add EDP and MP cue configs to deploy PanelMatch Daemon

[yunyeng]

This change is 

[SanjayVas]

Reviewed 10 of 10 files at r1, all commit messages.
Reviewable status: all files reviewed, 8 unresolved discussions (waiting on @yunyeng)

---

src/main/k8s/base.cue, line 73 at r1 (raw file):

			spec: {
				nodeSelector: {
					"iam.gke.io/gke-metadata-server-enabled": "true"

What is this for?

---

src/main/k8s/dev/BUILD.bazel, line 9 at r1 (raw file):

    srcs = ["edp_example_daemon_gke.cue"],
    cue_tags = {
        "party_type": "DATA_PROVIDER",

nit: can probably hard-code this in edp_example_daemon_gke.cue, given that it's a file with EDP in the name.

---

src/main/k8s/dev/BUILD.bazel, line 58 at r1 (raw file):

filegroup(
    name = "k8s_deployment_config",
    srcs = [":edp_example_daemon_gke.yaml"],

I'm surprised this works after 049d20f. If you run into any issues, you can drop the .yaml extension as that's the only file that the target outputs.

Code quote:

.yaml

---

src/main/k8s/dev/edp_example_daemon_gke.cue, line 20 at r1 (raw file):

package k8s

_container_registry:  string @tag("container_registry")

nit: for new files, let's try to be consistent with naming and use lowerCamelCase for fields.

Code quote:

_container_registry

---

src/main/k8s/dev/edp_example_daemon_gke.cue, line 23 at r1 (raw file):

_image_repo_prefix:   string @tag("image_repo_prefix")
_secret_name:         string @tag("secret_name")
_daemon_id:						string @tag("daemon_id")

Run cue fmt.

---

src/main/k8s/dev/edp_example_daemon_gke.cue, line 36 at r1 (raw file):

_private_ca_location_flag: "--privateca-ca-location=\(_private_ca_location)"

#GloudProject:            "ads-open-measurement"

I thought "dev" meant halo-cmm-dev project?

Code quote:

"ads-open-measurement"

---

src/main/kotlin/org/wfanet/panelmatch/client/deploy/example/gcloud/GoogleCloudExampleDaemonMain.kt, line 75 at r1 (raw file):

  @Option(names = ["--tink-credential-path"], description = ["KMS URI for Tink"], required = true)
  lateinit var tinkCredentialPath: String

Don't think this should be required in the event that the operator can get ACD working.

---

src/main/kotlin/org/wfanet/panelmatch/client/deploy/example/gcloud/GoogleCloudExampleDaemonMain.kt, line 97 at r1 (raw file):

  /** This can be customized per deployment. */
  private val defaults by lazy {
    GcpKmsClient.register(Optional.of(tinkFlags.tinkKeyUri), Optional.empty())

Shouldn't this use the credential path if specified?

Code quote:

Optional.empty()

---

src/main/k8s/dev/mp_example_daemon_gke.cue, line 76 at r1 (raw file):

		_private_ca_location_flag,
		"--id=\(_daemon_id)",
		"--tls-cert-file=/var/run/secrets/files/mc_tls.pem",

Did you not want to specify a different test cert for MP? Using the cert for the MC reporting frontend seems odd.

[stevenwarejones]

Reviewed 10 of 10 files at r1, all commit messages.
Reviewable status: all files reviewed, 8 unresolved discussions (waiting on @yunyeng)

[yunyeng]

Reviewable status: 1 of 16 files reviewed, 7 unresolved discussions (waiting on @SanjayVas, @stevenwarejones, and @yunyeng)

---

src/main/k8s/base.cue, line 73 at r1 (raw file):

Previously, SanjayVas (Sanjay Vasandani) wrote…

What is this for?

Done.

---

src/main/k8s/dev/edp_example_daemon_gke.cue, line 23 at r1 (raw file):

Previously, SanjayVas (Sanjay Vasandani) wrote…

Run cue fmt.

Done.

---

src/main/k8s/dev/edp_example_daemon_gke.cue, line 36 at r1 (raw file):

Previously, SanjayVas (Sanjay Vasandani) wrote…

I thought "dev" meant halo-cmm-dev project?

Done.

---

src/main/kotlin/org/wfanet/panelmatch/client/deploy/example/gcloud/GoogleCloudExampleDaemonMain.kt, line 75 at r1 (raw file):

Previously, SanjayVas (Sanjay Vasandani) wrote…

Don't think this should be required in the event that the operator can get ACD working.

Done.

---

src/main/kotlin/org/wfanet/panelmatch/client/deploy/example/gcloud/GoogleCloudExampleDaemonMain.kt, line 97 at r1 (raw file):

Previously, SanjayVas (Sanjay Vasandani) wrote…

Shouldn't this use the credential path if specified?

It looks at the environment variable actually which is set in base.cue

---

src/main/k8s/dev/mp_example_daemon_gke.cue, line 76 at r1 (raw file):

Previously, SanjayVas (Sanjay Vasandani) wrote…

Did you not want to specify a different test cert for MP? Using the cert for the MC reporting frontend seems odd.

Done.

[SanjayVas]

Reviewed 14 of 15 files at r2, all commit messages.
Reviewable status: 15 of 16 files reviewed, 8 unresolved discussions (waiting on @SanjayVas, @stevenwarejones, and @yunyeng)

---

src/main/k8s/dev/BUILD.bazel, line 17 at r2 (raw file):

        "tink_key_uri": TEST_GOOGLE_CLOUD_SETTINGS.tink_key_uri,
        "cloud_credentials_path": TEST_GOOGLE_CLOUD_SETTINGS.cloud_credentials_path,
        "private_ca_name": "20220217-zm6-cbh",

Sorry, I know I gave some potentially conflicting advice before I understood how this is all being used. Here's the general guidelines:

1. The dev package is for a specific GKE project, i.e. halo-cmm-dev. Anything that is for that environment can be hard-coded in the respective CUE file. Anyone who chooses to base their config on our dev one will need to change these accordingly.
2. Anything that needs to be different per-target that uses the same cue file (e.g. if you had separate targets to set up multiple EDP daemons) should be a CUE tag, but not a Make var. I don't think you have this use case here.
3. Anything that might change per-run (e.g. secret names) should be a Make var.

Code quote:

        "private_ca_name": "20220217-zm6-cbh",
        "private_ca_pool_id": "SomeCommonName",
        "private_ca_location": "us-central1",

---

src/main/k8s/dev/edp_example_daemon_gke.cue, line 24 at r2 (raw file):

_cloud_storage_bucket:   string @tag("cloud_storage_bucket")
_tink_key_uri:           string @tag("tink_key_uri")
_cloud_credentials_path: string @tag("cloud_credentials_path")

Shouldn't this be a hard-coded path within the K8s secret?

Code quote:

@tag("cloud_credentials_path")

---

src/main/k8s/dev/edp_example_daemon_gke.cue, line 53 at r2 (raw file):

_tink_key_uri_flags: [
	"--tink-key-uri=\(_tink_key_uri)",
	"--tink-credential-path=\(_cloud_credentials_path)",

Why are we passing this both in a flag and in the environment variable?

Code quote:

--tink-credential-path

---

src/main/k8s/dev/edp_example_daemon_gke.cue, line 57 at r2 (raw file):

_exchange_api_flags: [
	"--exchange-api-target=" + (#Target & {name: "v2alpha-public-api-server"}).target,

1. This only works if the Kingdom is in the same cluster
2. We already have a definition for the Kingdom public API target: #KingdomPublicApiTarget

Code quote:

(#Target & {name: "v2alpha-public-api-server"}).target

---

src/main/kotlin/org/wfanet/panelmatch/client/deploy/ExchangeWorkflowDaemon.kt, line 122 at r2 (raw file):

  /** Runs [exchangeStepLauncher] in an infinite loop. */
  protected open suspend fun runDaemon(exchangeStepLauncher: ExchangeStepLauncher) {
    scope.launch {

Suspend functions shouldn't normally launch things in a different scope. They should suspend until complete.

If you have function that launches something in the background and doesn't wait for it complete, it shouldn't be a suspend function.

Maybe what you want is for this to return the Job returned by launch, so that run can block on it?

e.g. in run:

val job = runDaemon(exchangeStepLauncher)
runBlocking { job.join() }

---

src/main/k8s/dev/mp_example_daemon_gke.cue, line 18 at r2 (raw file):

// src/main/k8s/example_daemon_from_cue.yaml

package k8s

This whole file looks very similar to the EDP one. Either extract the commonalities to a separate cue_library, or have the differences be cue tags that you specify in the Bazel target.

[stevenwarejones]

Reviewed 15 of 15 files at r2, all commit messages.
Reviewable status: all files reviewed, 8 unresolved discussions (waiting on @SanjayVas and @yunyeng)

[stevenwarejones]

Reviewed 4 of 4 files at r3, all commit messages.
Reviewable status: all files reviewed, 8 unresolved discussions (waiting on @SanjayVas and @yunyeng)

[SanjayVas]

Can this PR be dropped? I believe it was superseded by a couple other PRs.