test: Create CUE files for Population Requisition Fulfiller KIND test

[jojijac0b]

Add PopulationRequisitionFulfillerDaemon image to images.bzl
Create the CUE files that will invoke the PopulationRequisitionFulfillerDaemon

[wfa-reviewable]

This change is 

[stevenwarejones]

Reviewed 2 of 5 files at r1, 3 of 7 files at r2.
Reviewable status: 5 of 11 files reviewed, 2 unresolved discussions (waiting on @jojijac0b and @Marco-Premier)

---

src/main/k8s/population_requisition_fulfiller.cue line 60 at r2 (raw file):

                "--event-message-descriptor-set=\(set)"
            }] + [ for config in _config.populationKeyAndInfoList {
                "--population-resource-name=\(config.populationResourceName)",

you don't really know this at start time, right?

---

src/main/kotlin/org/wfanet/measurement/loadtest/resourcesetup/ResourceSetupRunner.kt line 117 at r2 (raw file):

        flags.outputDir,
      )
      .process(dataProviderContents, measurementConsumerContent, duchyCerts, pdpContent)

nit: can you rename dataProviderContents to edpContents?

[stevenwarejones]

Reviewed 4 of 6 files at r3, 5 of 12 files at r4, 13 of 17 files at r5, 9 of 9 files at r7, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @jojijac0b, @Marco-Premier, and @SanjayVas)

---

src/main/kotlin/org/wfanet/measurement/dataprovider/RequisitionFulfiller.kt line 219 at r7 (raw file):

      val a = requisitionsStub.listRequisitions(request).requisitionsList
      println("joji in rf: $a")
      return a

you can revert this change

---

src/test/kotlin/org/wfanet/measurement/integration/k8s/EmptyClusterCorrectnessTest.kt line 142 at r7 (raw file):

        var populationDataProviderCert: String? = null
        var modelLine: String? = null
        val dataProviders = mutableMapOf<String, Resources.Resource>()

nit: can you rename this to eventDataProviders

---

src/test/kotlin/org/wfanet/measurement/integration/k8s/EmptyClusterCorrectnessTest.kt line 152 at r7 (raw file):

              measurementConsumerCert = resource.measurementConsumer.certificate
            }
            Resources.Resource.ResourceCase.DATA_PROVIDER -> {

ditto on the rename

[SanjayVas]

Reviewed 3 of 12 files at r4, 1 of 17 files at r5, 4 of 9 files at r7, all commit messages.
Reviewable status: all files reviewed, 12 unresolved discussions (waiting on @jojijac0b, @Marco-Premier, and @tristanvuong2021)

---

a discussion (no related file):
Move the changes not related to adding the CUE files (e.g. adding the pop fulfiller to the correctness test, changes to the spec converter, updating resource setup) to separate PRs. Make sure to stack your PRs accordingly if there are dependencies (have the PR branches and PRs based appropriately)

---

src/main/k8s/dev/BUILD.bazel line 402 at r7 (raw file):

    testonly = True,
    srcs = [
        "synthetic_generator_config_files_kustomization.yaml",

Why would the EDP simulator need these additional files?

---

src/main/k8s/dev/BUILD.bazel line 428 at r7 (raw file):

    ],
)

I don't see a CUE file or matching rules for the Population requisition fulfiller here like there is in local. There needs to be one.

---

src/main/k8s/dev/BUILD.bazel line 457 at r7 (raw file):

    srcs = [
        "reporting_v2_config_files_kustomization.yaml",
        "//src/main/k8s/testing/data:real_population_spec_large",

Same here. Reporting shouldn't need these additional files.

---

src/main/k8s/testing/secretfiles/BUILD.bazel line 136 at r7 (raw file):

)

proto_descriptor_set(

You shouldn't need this, as proto_descriptor_set generates a descriptor set for the transitive dependencies as well. Therefore the descriptor set for the test event message will contain this.

---

src/main/k8s/testing/secretfiles/BUILD.bazel line 144 at r7 (raw file):

proto_descriptor_set(
    name = "test_event_type_set",

nit: It looks like you copied the naming from the target for the set of known EventGroup metadata types. Here, the concept is the descriptor set for the event message called TestEvent.

Suggestion:

test_event_message_descriptor_set

---

src/main/k8s/population_requisition_fulfiller.cue line 54 at r7 (raw file):

    _eventDescriptorFlags: {
        let flagLists = [ for file in _config.eventMessageDescriptorSet {[

This one is just one flag, so I don't think you need a list of lists.

---

src/main/k8s/local/population_requisition_fulfillers.cue line 24 at r7 (raw file):

_populationSpec:           "/etc/\(#AppName)/config-files/population_spec_large.textproto"

This must be a spec that matches //src/main/k8s/testing/data:synthetic_generation_specs_small in the local config.

---

src/main/k8s/local/population_requisition_fulfillers.cue line 45 at r7 (raw file):

	[ for fulfiller in populationRequisitionFulfillers {fulfiller.networkPolicies}]

populationRequisitionFulfillers: {

We're only going to have one PDP deployed by each operator in each environment, so everything here should be singular (the CUE field, the file name, the Bazel target, etc.)

---

src/main/docker/images.bzl line 107 at r7 (raw file):

        name = "population_requisition_fulfiller_image",
        image = "//src/main/kotlin/org/wfanet/measurement/populationdataprovider:population_requisition_fulfiller_daemon_image",
        repository = _PREFIX + "/measurement/population-requisition-fulfiller",

I think data-provider would be a clearer component here, as this is something any Population DataProvider could run.

Suggestion:

data-provider

[SanjayVas]

Reviewable status: all files reviewed, 12 unresolved discussions (waiting on @jojijac0b, @Marco-Premier, and @tristanvuong2021)

---

src/main/k8s/dev/BUILD.bazel line 428 at r7 (raw file):

Previously, SanjayVas (Sanjay Vasandani) wrote…

I don't see a CUE file or matching rules for the Population requisition fulfiller here like there is in local. There needs to be one.

Sorry, I see this PR is just covering local cluster. The changes to the dev config can be in a later PR.

[jojijac0b]

Reviewable status: 30 of 33 files reviewed, 12 unresolved discussions (waiting on @Marco-Premier, @SanjayVas, @stevenwarejones, and @tristanvuong2021)

---

src/main/docker/images.bzl line 107 at r7 (raw file):

Previously, SanjayVas (Sanjay Vasandani) wrote…

I think data-provider would be a clearer component here, as this is something any Population DataProvider could run.

Done.

---

src/main/k8s/dev/BUILD.bazel line 402 at r7 (raw file):

Previously, SanjayVas (Sanjay Vasandani) wrote…

Why would the EDP simulator need these additional files?

Removed

---

src/main/k8s/dev/BUILD.bazel line 457 at r7 (raw file):

Previously, SanjayVas (Sanjay Vasandani) wrote…

Same here. Reporting shouldn't need these additional files.

Done.

[SanjayVas]

Reviewed 1 of 12 files at r4, 2 of 3 files at r8, 1 of 10 files at r9, all commit messages.
Reviewable status: 25 of 34 files reviewed, 12 unresolved discussions (waiting on @jojijac0b, @Marco-Premier, @stevenwarejones, and @tristanvuong2021)

---

a discussion (no related file):

Previously, SanjayVas (Sanjay Vasandani) wrote…

Move the changes not related to adding the CUE files (e.g. adding the pop fulfiller to the correctness test, changes to the spec converter, updating resource setup) to separate PRs. Make sure to stack your PRs accordingly if there are dependencies (have the PR branches and PRs based appropriately)

Holding off on reviewing further until this is done.

---

build/variables.bzl line 62 at r9 (raw file):

    mc_config_secret_name = "$(k8s_mc_config_secret_name)",
    grafana_secret_name = "$(k8s_grafana_secret_name)",
    pdp1_name = "$(pdp1_name)",

nit: you don't need to number these as I assume we'll only have one PDP for our test environments. We have multiple EDPs since multi-EDP measurements behave differently than single-EDP ones.

---

src/main/k8s/dev/synthetic_generator_config_files_kustomization.yaml line 19 at r9 (raw file):

  files:
  - synthetic_population_spec_large.textproto
  - population_spec_small.textproto

Revert this since you reverted the corresponding changes in the BUILD file

---

src/main/k8s/dev/config_files_kustomization.yaml line 19 at r9 (raw file):

  files:
  - authority_key_identifier_to_principal_map.textproto
  - synthetic_population_spec_small.textproto

Dev doesn't use the small spec.

[jojijac0b]

Reviewable status: 19 of 34 files reviewed, 11 unresolved discussions (waiting on @Marco-Premier, @SanjayVas, @stevenwarejones, and @tristanvuong2021)

---

a discussion (no related file):

Previously, SanjayVas (Sanjay Vasandani) wrote…

Holding off on reviewing further until this is done.

Split up this PR into 3 new PRs. I've made code changes to all the comments that I've responded to in this PR. We can move the conversation to the new PRs

PR1: Creates CUE files and daemon image

• feat: Create local Kubernetes configuration for Population Requisition fulfiller #2012

PR2: Updates PopulationSpecConverter

• Update population spec converter #2013

PR3: Creates resources and adds PopulationRequisitionFulfiller test to EmptyClusterCorrectnessTest

• Create Population Requisition Fulfiller KIND test and resources #2014

---

src/main/k8s/dev/config_files_kustomization.yaml line 19 at r9 (raw file):

Previously, SanjayVas (Sanjay Vasandani) wrote…

Dev doesn't use the small spec.

Done.

---

src/main/k8s/dev/synthetic_generator_config_files_kustomization.yaml line 19 at r9 (raw file):

Previously, SanjayVas (Sanjay Vasandani) wrote…

Revert this since you reverted the corresponding changes in the BUILD file

Done.

---

src/main/k8s/local/population_requisition_fulfillers.cue line 24 at r7 (raw file):

Previously, SanjayVas (Sanjay Vasandani) wrote…

This must be a spec that matches //src/main/k8s/testing/data:synthetic_generation_specs_small in the local config.

changes population_spec_large to population_spec_small

---

src/main/k8s/local/population_requisition_fulfillers.cue line 45 at r7 (raw file):

Previously, SanjayVas (Sanjay Vasandani) wrote…

We're only going to have one PDP deployed by each operator in each environment, so everything here should be singular (the CUE field, the file name, the Bazel target, etc.)

I think it may be valuable to keep this plural to leave room for multiple pdps to be deployed. @stevenwarejones

---

src/main/k8s/testing/secretfiles/BUILD.bazel line 144 at r7 (raw file):

Previously, SanjayVas (Sanjay Vasandani) wrote…

nit: It looks like you copied the naming from the target for the set of known EventGroup metadata types. Here, the concept is the descriptor set for the event message called TestEvent.

Done.

---

src/main/kotlin/org/wfanet/measurement/dataprovider/RequisitionFulfiller.kt line 219 at r7 (raw file):

Previously, stevenwarejones (Steven Ware Jones) wrote…

you can revert this change

Done.

[stevenwarejones]

Reviewed 1 of 3 files at r8, 2 of 10 files at r9, 11 of 11 files at r10, 1 of 1 files at r11, 1 of 1 files at r12, all commit messages.
Reviewable status: all files reviewed, 10 unresolved discussions (waiting on @jojijac0b, @Marco-Premier, @SanjayVas, and @tristanvuong2021)

---

build/variables.bzl line 62 at r9 (raw file):

Previously, SanjayVas (Sanjay Vasandani) wrote…

nit: you don't need to number these as I assume we'll only have one PDP for our test environments. We have multiple EDPs since multi-EDP measurements behave differently than single-EDP ones.

i think this is fine for test. The benefit of keeping the numbering is that it clearly communicates there can be more than one.

---

src/main/k8s/local/population_requisition_fulfillers.cue line 45 at r7 (raw file):

Previously, jojijac0b wrote…

I think it may be valuable to keep this plural to leave room for multiple pdps to be deployed. @stevenwarejones

I'd prefer to keep it plural, if possible. The system supports multiple. A multi-region Kingdom would definitely need multiple, for example.

[SanjayVas]

Reviewed 1 of 1 files at r11, all commit messages.
Reviewable status: all files reviewed, 10 unresolved discussions (waiting on @jojijac0b, @Marco-Premier, and @tristanvuong2021)

---

a discussion (no related file):

Previously, jojijac0b wrote…

Split up this PR into 3 new PRs. I've made code changes to all the comments that I've responded to in this PR. We can move the conversation to the new PRs

PR1: Creates CUE files and daemon image

• feat: Create local Kubernetes configuration for Population Requisition fulfiller #2012

PR2: Updates PopulationSpecConverter

• Update population spec converter #2013

PR3: Creates resources and adds PopulationRequisitionFulfiller test to EmptyClusterCorrectnessTest

• Create Population Requisition Fulfiller KIND test and resources #2014

I rather wish you had just made this PR be the one for the local K8s config since most of the comments were pertaining to that. It would have made it much easier to maintain continuity of comment threads. Regardless, I suppose this PR can now be closed.