[PF-308] Add GCS bucket and bigquery data references by zloery · Pull Request #212 · DataBiosphere/terra-workspace-manager

zloery · 2021-02-01T19:27:26Z

pair programmed with @jaycarlton

This change adds support for GCS bucket and BigQuery dataset data references in addition to the existing Data Repo snapshot references. It also introduces a new pattern for reference endpoints broken out by reference type. Cleanup of the old endpoints after migration from the old API to the new API is tracked in PF-404.

To preserve backwards compatibility, the existing endpoints and interface objects are left unchanged. This means the existing GET and enumerate endpoints will not work with bucket and dataset references, as that would require changing the response object.

This includes basic reference validation for the new types, similar to what we do for TDR snapshots. In this case we make a GET call for the specified bucket or dataset on behalf of the user, which requires that the user have appropriate permission on the object. This does not do any further validation, such as examining bucket contents or BQ tables.

This change is not feature gated behind the MC_WORKSPACE. Rawls will have access to these new reference types.

This requires publishing a new client library version, which I'll do after approvals but before merging.

jaycarlton

some notes

build.gradle

src/main/java/bio/terra/workspace/service/crl/CrlService.java

src/main/java/bio/terra/workspace/service/datareference/DataReferenceService.java

jaycarlton · 2021-02-01T21:43:45Z

src/main/java/bio/terra/workspace/service/datareference/DataReferenceService.java

+          referenceRequest.workspaceId(),
+          referenceRequest.referenceType(),
+          referenceRequest.name());
+      throw new DuplicateDataReferenceException(


it would be good to include the name and type and workspace iD in the error message.

My concern with that is that name is a user provided string, and the input from security has been to avoid directly returning user-provided strings where possible. A common pattern we use instead is returning an error message without the values and logging the actual values, which I've added here.

Sounds good.

We will be returning the name string in many cases. It is the user's name for the object and we will need to return it in get and enumerate requests. If we need to constrain the contents of the name, we should do that. That seems to be Google's approach; for example, BigQuery dataset naming

And there is nothing dangerous about returning the reference type or workspaceId is there?
So if we fixed all of that, you could skip the logging and just throw and let the global exception handler do the logging.

Makes sense, I'll add bucket and dataset name validation and then return the names as part of the error response.

src/main/java/bio/terra/workspace/service/datareference/model/BigQueryDatasetReference.java

src/main/java/bio/terra/workspace/service/datareference/utils/DataReferenceValidationUtils.java

jaycarlton · 2021-02-01T21:54:06Z

src/test/java/bio/terra/workspace/common/utils/ControllerValidationUtilsTest.java

+        new DataRepoSnapshot().snapshot("snapshot-name").instanceName("instance-name");
+    CreateDataReferenceRequestBody deprecatedRequest =
+        new CreateDataReferenceRequestBody()
+            .referenceType(ReferenceTypeEnum.DATA_REPO_SNAPSHOT)


I think it's not too late to rename ReferenceTypeEnum to ReferenceType. Clients have access to that name but since the ordinals are the same it should just work, right?

src/test/java/bio/terra/workspace/service/datareference/DataReferenceServiceTest.java

src/test/java/bio/terra/workspace/service/datareference/model/ReferenceObjectTest.java

jaycarlton

Thanks for the detailed responses!

davidangb · 2021-02-02T20:39:45Z

fyi @MatthewBemis for BigQuery references and @zarsky-broad for APIs related to managing references

davidangb

👍 overall, some non-blocking comments inline.

davidangb · 2021-02-02T21:09:30Z

src/main/java/bio/terra/workspace/service/datareference/model/DataReference.java

+    switch (referenceType()) {
+      case DATA_REPO_SNAPSHOT:
+        // TODO: we populate both the deprecated reference field and the new referenceInfo fields
+        // until all clients have migrated to use the new field.


this will require more than just adopting the new client library - for instance, the Terra UI is looking inside the reference key to find snapshot ids. We can migrate but it'll be more work.

Hm, good to know. I think we would still want to migrate, but it sounds like it's going to be worth coordinating more on this. I've currently been tracking this as PF-404, but we may end up with AS tickets as well.

davidangb · 2021-02-02T21:14:34Z

src/main/java/bio/terra/workspace/service/datareference/utils/DataReferenceValidationUtils.java

+    } catch (BigQueryException e) {
+      throw new InvalidDataReferenceException("Error while trying to access BigQuery dataset", e);
+    }
+  }


I wonder if each ReferenceObject subclass should define their own validate() method inside their own class - similar to how toJson() is defined. Then we'd just delegate to their own implementations. This might require passing a CrlService or DataRepoService to it.

Having to pass the services to the objects was the reason I didn't want to take that approach - it feels like we're drifting further away from these being value classes. (They already kind of weren't with things like toApiModel(), but adding more members makes serialization/deserialization trickier).

What is the benefit of them being value classes if we have to write switch statements to dispatch methods on them?

I think it would be a good separation of concerns to have the reference objects encapsulate the knowledge of the cloud resource they reference.

The value is that they can be serialized into the database and back out, which is how we store the actual references. If we require the reference objects to have a DataRepoService field, that gets more complicated - do we serialize the DataRepoService field with the reference? and if not, how do you deserialize into a valid reference object?

I think there is a middle ground. I wrote a doc for further discussion: Workspace Manager Object Hierarchy

davidangb · 2021-02-02T21:15:25Z

src/main/resources/api/service_openapi.yaml

-      # PLACEHOLDER_VALUE should be removed when we have an actual second value
-      # to use.
-      enum: ['DATA_REPO_SNAPSHOT', 'PLACEHOLDER_VALUE'] #eventually include GCS bucket, etc.
+      enum: ['DATA_REPO_SNAPSHOT', 'GOOGLE_BUCKET', 'BIG_QUERY_DATASET']


glad to have PLACEHOLDER_VALUE out of here!

Same, that truncation is still one of the strangest opinions I've seen swagger take.

davidangb · 2021-02-02T21:16:52Z

src/main/resources/api/service_openapi.yaml

+#          type: string
+#        datasetName:
+#          description: The name of this dataset
+#          type: string


delete this block?

whoops, good catch

ddietterich

I think there is an upward compatibility issue and maybe a validation issue.

ddietterich · 2021-02-02T21:19:59Z

src/main/resources/api/service_openapi.yaml

          description: The ID of the Data Repo snapshot
          type: string

+#    GoogleBucket:


Are these needed?

Nope, we replaced those definitions with shared ones from the common library and I forgot to remove them

ddietterich · 2021-02-02T21:35:04Z

src/main/resources/api/service_openapi.yaml

    CreateDataReferenceRequestBody:
      type: object
-      required: [name, cloningInstructions]
+      required: [name, referenceType, cloningInstructions]


Does that cause an upward compatibility problem with Rawls?
If they can specify the type, then they can also change from using reference to using referenceInfo.

Otherwise, I think we need to leave it optional - maybe with a default value?

I think that's a good point. default it to data repo snapshot and leave it optional.

Rawls is already specifying type, and this was previously enforced by rejecting requests with no referenceType field in ControllerValidationUtils.
We did that validation as part of the app logic instead of putting it in Swagger because the request shape was (kind of) also used for controlled resource requests, so only common fields between the two were made required. Now that we're removing all the unused controlled resource stuff here, we can move referenceType to required. I'd also like to move referenceInfo there once we stop using the reference field.

ddietterich · 2021-02-02T21:35:46Z

src/main/java/bio/terra/workspace/app/controller/WorkspaceApiController.java

+    final DataReferenceInfo referenceInfo = body.getReferenceInfo();
+    final ReferenceObject referenceObject;
+    if (body.getReference() != null) {
+      referenceObject =


It looks like the referenceType is required in the OpenAPI yaml, so should we verify that it is set to data repo snapshot in this case?

I see later that there is a request validator, but it will error if the referenceType is not specified which seems incompatible.

We also previously errored in ControllerValidationUtils if referenceType was not specified, so that is actually compatible.

ddietterich · 2021-02-02T21:45:44Z

src/main/java/bio/terra/workspace/common/utils/ControllerValidationUtils.java

+    // TODO(PF-404): requests using the deprecated reference field will not set the referenceInfo
+    // field. referenceInfo can be made required and this check can be removed once clients migrate.
+    DataReferenceInfo info =
+        body.getReferenceInfo() == null ? new DataReferenceInfo() : body.getReferenceInfo();


But that reference is not filled in with anything. Either this is broken or I am confused...

This handles the existing case where a client specifies the old reference field but does not specify the new referenceInfo field. You can't have bucket or dataset references this way (as getGoogleBucket() and getBigQueryDataset() on this reference will be null), but you can specify a snapshot in the old reference field this way, and we want to support that until we move clients. That's why we check either the referenceInfo or the reference field when validating snapshots here.
(As above, existing clients do already use the DATA_REPO_SNAPSHOT referenceType, so that hasn't changed)

ddietterich · 2021-02-02T21:51:35Z

src/main/java/bio/terra/workspace/service/datareference/DataReferenceService.java

+          referenceRequest.workspaceId(),
+          referenceRequest.referenceType(),
+          referenceRequest.name());
+      throw new DuplicateDataReferenceException(


We will be returning the name string in many cases. It is the user's name for the object and we will need to return it in get and enumerate requests. If we need to constrain the contents of the name, we should do that. That seems to be Google's approach; for example, BigQuery dataset naming

ddietterich · 2021-02-02T22:01:09Z

src/main/java/bio/terra/workspace/service/datareference/DataReferenceService.java

+          referenceRequest.workspaceId(),
+          referenceRequest.referenceType(),
+          referenceRequest.name());
+      throw new DuplicateDataReferenceException(


And there is nothing dangerous about returning the reference type or workspaceId is there?
So if we fixed all of that, you could skip the logging and just throw and let the global exception handler do the logging.

src/main/java/bio/terra/workspace/service/datareference/model/BigQueryDatasetReference.java

ddietterich · 2021-02-02T22:18:13Z

src/main/java/bio/terra/workspace/service/datareference/model/DataReferenceType.java

+    typeMap.put(DataReferenceType.GOOGLE_BUCKET, ReferenceTypeEnum.GOOGLE_BUCKET);
+    typeMap.put(DataReferenceType.BIG_QUERY_DATASET, ReferenceTypeEnum.BIG_QUERY_DATASET);

    sqlMap.put(DATA_REPO_SNAPSHOT, "DATA_REPO_SNAPSHOT");


This map is unnecessary. The Enum's built-in name() and validOf methods will do the same thing. And you can convert to them without changing your interface.

The idea of the sql map is to have invariant names, so that even if they refactor the enum names in Java, we can serialize correctly. .name() is too smart for this purpose, and would return the new name.

Cutting out the typeMap seems fine. I'm a little less sure about sqlMap for the reason Jay stated, since this value does get serialized directly.

ddietterich · 2021-02-02T22:19:37Z

src/main/java/bio/terra/workspace/service/datareference/model/DataReferenceType.java


  static {
    typeMap.put(DataReferenceType.DATA_REPO_SNAPSHOT, ReferenceTypeEnum.DATA_REPO_SNAPSHOT);
+    typeMap.put(DataReferenceType.GOOGLE_BUCKET, ReferenceTypeEnum.GOOGLE_BUCKET);


And this map is overkill. Maybe I will write you an "enum" class that will use a list search - which is generally faster than a hash for lists smaller than 10 items...

I know, it probably isn't important and it is all static. If we were running on a PDP-11/70 you would thank me :)

https://skn.noip.me/pdp11/pdp11.html

ddietterich · 2021-02-02T22:37:25Z

src/main/java/bio/terra/workspace/service/datareference/utils/DataReferenceValidationUtils.java

+    } catch (BigQueryException e) {
+      throw new InvalidDataReferenceException("Error while trying to access BigQuery dataset", e);
+    }
+  }


What is the benefit of them being value classes if we have to write switch statements to dispatch methods on them?

I think it would be a good separation of concerns to have the reference objects encapsulate the knowledge of the cloud resource they reference.

jaycarlton · 2021-02-02T23:01:14Z

#212 (comment)

Yeah, and we could have a parent class with just the type as a public member in case we need it.

ddietterich · 2021-02-03T18:46:29Z

As posted in slack:

Small brainstorm especially of interest to @zloery and @jaycarlton. What about making the new references endpoints use the path /api/workspaces/v1/resources/references and keep all of the WM resources within one collection. We would leave the current datareferences as is for backward compatibility.

It might make the new version cleaner because we could require referenceType and remove the reference field. And we can make a general GET for /resources that returns everything we’ve got. I know this is a late-breaking change, but I just thought of it after Jay’s suggestion on the controlled resources spec.

zloery · 2021-02-03T21:16:05Z

re: moving to /api/workspaces/v1/resources/references, I'm definitely thinking about it.

I'm also thinking about our model of resources - the DB structure suggests that we have data references to all resources (both controlled and uncontrolled), so should the common GET be on .../datareferences? Some parts of that don't really make sense though, e.g. I don't know what the referenceId of a reference to a controlled resource really means. probably worth more discussion

zloery · 2021-02-04T19:26:24Z

Per earlier discussion, we'll break out each type into a separate endpoint, while still supporting the existing data reference APIs for compatibility in the short-term. Moving this to a draft while I work on that

zarsky-broad · 2021-02-04T21:37:58Z

FWIW: OpenAPITools/openapi-generator#5661 (comment)

zloery · 2021-02-04T22:14:35Z

...huh. I'm wary of blocking on something that's been in progress for almost a year, but I want to keep an eye on that. thanks for the pointer!

gjuggler · 2021-02-05T15:01:55Z

...huh. I'm wary of blocking on something that's been in progress for almost a year, but I want to keep an eye on that. thanks for the pointer!

Also, don't we use swagger-codegen and not openapi-generator? ::shakes fist at whoever forked the projects, however good their intentions may have been::

zloery · 2021-02-08T20:22:00Z

Update: I've broken out the create, get, get-by-name, and delete endpoints for each resource type separately, and the new interface no longer uses any polymorphic types. I'm not totally sure this structure is right (delete and get-by-id don't really depend on type and could be methods on /api/workspaces/v1/{id}/resources/references, but I think this is ready for more eyes.
I haven't replaced the enumerate endpoint yet, which we will need to do before fully migrating. I think that's out of scope for this PR

ddietterich

lotta good stuff in here! I want to discuss object model before we merge on it and maybe remove some endpoints.

src/main/java/bio/terra/workspace/app/controller/DataRepoReferenceController.java

ddietterich · 2021-02-09T16:22:09Z

src/main/java/bio/terra/workspace/service/datareference/utils/DataReferenceValidationUtils.java

+    } catch (BigQueryException e) {
+      throw new InvalidDataReferenceException("Error while trying to access BigQuery dataset", e);
+    }
+  }


I think there is a middle ground. I wrote a doc for further discussion: Workspace Manager Object Hierarchy

src/main/resources/api/service_openapi.yaml

ddietterich · 2021-02-09T16:51:15Z

src/main/resources/api/service_openapi.yaml

+        '500':
+          $ref: '#/components/responses/ServerError'
+
+  /api/workspaces/v1/{id}/resources/references/datarepo/snapshot/name/{name}:


Is name URL safe? Or you just assume it is encoded?

Names must be URL safe and they're checked for validity in Controller methods

davidangb

see comments inline!

davidangb · 2021-02-10T16:34:25Z

src/main/java/bio/terra/workspace/app/controller/DataRepoReferenceController.java

+            .cloningInstructions(
+                CloningInstructions.fromApiModel(body.getMetadata().getCloningInstructions()))
+            .referenceObject(referenceObject)
+            .build();


this needs a rebase now that #221 merged; this should include .referenceDescription(body.getReferenceDescription())

davidangb · 2021-02-10T16:35:26Z

src/main/java/bio/terra/workspace/app/controller/DataRepoReferenceController.java

+            referenceId.toString(), id.toString(), userReq.getEmail()));
+    return new ResponseEntity<>(HttpStatus.NO_CONTENT);
+  }
+}


also from #221 this needs to include updateDataReference(...)

davidangb · 2021-02-10T16:51:40Z

src/main/java/bio/terra/workspace/app/controller/GoogleReferenceController.java

+                    .toApiModel())
+            .metadata(ControllerTranslationUtils.metadataFromDataReference(ref));
+    return new ResponseEntity<>(response, HttpStatus.OK);
+  }


similar to private void deleteDataReference and private DataReference createDataReference, could the get-by-id and get-by-name methods be factored into a common method?

davidangb · 2021-02-10T16:56:04Z

src/main/java/bio/terra/workspace/app/controller/WorkspaceApiController.java

+      // TODO(PF-404): this is a workaround until clients migrate off this endpoint.
+      if (ref.referenceType() == DataReferenceType.DATA_REPO_SNAPSHOT) {
+        responseList.addResourcesItem(ref.toApiModel());
+      }


If this omits non-snapshot references from its response payload, it could easily return incorrect pagination - user requested a limit of 5, but we only return 3.

I don't think this is worth fixing in this PR - we should encourage clients to move to the new endpoints instead. But let's remain aware of this behavior in case it causes strange problems elsewhere, potentially in automated tests.

Agreed. I think the main way to mitigate this is not adopting the new endpoints (including creating non-snapshot references) until the enumerate is ready, but that's a risk.

davidangb · 2021-02-10T16:58:41Z

src/main/java/bio/terra/workspace/service/datareference/utils/DataReferenceValidationUtils.java

+   */
+  public static final Pattern BQ_DATASET_NAME_VALIDATION_PATTERN =
+      Pattern.compile("^[_a-zA-Z0-9]{1,1024}$");
+


do any of the Google client libs offer name validation that we could delegate to, instead of writing our own regexes? (honest question, I don't know if they do or not)

Generally the client libs don't validate the names, you'll just get a 40X response from GCP. Agreed that rolling our own regexes for a "necessary but not sufficient" check is a little messy, but I think it's better than passing around totally unverified strings

Devil's advocate: what are we ever going to use the string for except to pass to Google to make a resource? What's the harm in late validation?

We want to send it back to the user in an error message. Security doesn't want us putting unvalidated user input into an output string.

davidangb · 2021-02-10T17:02:25Z

src/main/java/bio/terra/workspace/service/datareference/utils/DataReferenceValidationUtils.java

+      throw new InvalidDataReferenceException(
+          String.format("Error while trying to access GCS bucket %s", ref.bucketName()), e);
+    }
+  }


I haven't looked at the innards of CRL - do you know what happens here if the bucket is requester-pays?

I haven't seen that case specifically, but CRL is a really thin wrapper around the Google client library. If we're calling GET on a bucket (which I think is a class B operation), it looks like this will fail with a 400 UserProjectMissing and we'd end up throwing. Is that a usecase we need to support?

davidangb · 2021-02-10T17:06:22Z

src/main/java/bio/terra/workspace/app/controller/GoogleReferenceController.java

+
+  private AuthenticatedUserRequest getAuthenticatedInfo() {
+    return authenticatedUserRequestFactory.from(request);
+  }


DataRepoReferenceController and GoogleReferenceController share boilerplate - is there opportunity for a ReferenceController base class?

Done. I'm not sure I took the cleanest approach given the way I'm passing beans around, but it does cut down on boilerplate. I might try to revisit this a bit to shrink the argument list

davidangb · 2021-02-10T17:08:01Z

src/main/resources/api/service_openapi.yaml


+  /api/workspaces/v1/{id}/resources/references/datarepo/snapshots:
+    parameters:
+    - $ref: '#/components/parameters/Id'


not for this PR: I really wish this was named workspaceId instead of the totally-ambiguous id

it really should be, that's tripped people up in client tests before when id gets mixed up between workspaces and references.

davidangb · 2021-02-10T17:10:31Z

src/main/resources/api/service_openapi.yaml

+        application/json:
+          schema:
+            $ref: '#/components/schemas/BigQueryDatasetReference'
+


this will also need updates based on #221 - adding description to DataReferenceMetadata ( I assume that's where it would go ), plus patch APIs for each reference type.

zloery · 2021-02-10T22:40:34Z

Update: the current approach is per-type create, patch, and get (and get-by-name) endpoints. Delete and enumerate will be shared at a .../resources level, out of scope for this PR. Reference descriptions are also merged in, PTAL.

ddietterich

Looking good. A scattering of comments.

I want to understand moving the untar of the common swagger into the source tree. I'd like to avoid that if possible

ddietterich · 2021-02-11T20:47:33Z

src/main/java/bio/terra/workspace/app/controller/GoogleReferenceController.java

+    GoogleBucketReference response =
+        new GoogleBucketReference()
+            .bucket(
+                ((bio.terra.workspace.service.datareference.model.GoogleBucketReference)


I think we should have some prefix or suffix like API on the generated code so we don't run into these name collisions.
Duplicate names will make it easy to screw up and confusing to understand and debug.

It's easy to set up a global prefix in Swagger.

Agreed, but I think making that change (especially if we add a global prefix) ends up out of scope for this PR.

ddietterich · 2021-02-11T20:51:28Z

src/main/java/bio/terra/workspace/app/controller/GoogleReferenceController.java

+        new GoogleBucketReference()
+            .bucket(
+                ((bio.terra.workspace.service.datareference.model.GoogleBucketReference)
+                        ref.referenceObject())


We can leave this for now, but this is where having the class hierarchy would make things cleaner.

ddietterich · 2021-02-11T21:04:09Z

src/main/java/bio/terra/workspace/app/controller/common/ReferenceController.java

+    }
+
+    logger.info(
+        String.format(


You don't need the almost all the time. You can use placeholders {} in the log message.

Ah, I missed this one. thanks!

ddietterich · 2021-02-11T21:07:57Z

build.gradle

+	from zipTree(configurations.cloudResourceSchema.singleFile).matching {
+		include 'cloud_resources_uid.yaml'
+	}
+	into "${resourceDir}/api/common"


Why do we need to unpack this into the source tree?
Is an alternative to copy our resources into the build tree?

It would be nice to deploy our yaml file into a build dir so we could use relative (non-upward) paths.

I unpacked it into the source tree so that our swagger-ui (also in the source dir) can parse references to it. otherwise, those schemas won't show up in swagger-ui, which breaks example values.

We can leave it for now. I'll make a ticket.
Another approach might be to move all of the parts into a build subdirectory and do the codegen from there.
I'll make a ticket.

jaycarlton · 2021-02-11T21:25:41Z

build.gradle

+	from zipTree(configurations.cloudResourceSchema.singleFile).matching {
+		include 'cloud_resources_uid.yaml'
+	}
+	into "${resourceDir}/api/common"


It would be nice to deploy our yaml file into a build dir so we could use relative (non-upward) paths.

jaycarlton · 2021-02-11T21:29:17Z

build.gradle

+
+// Internal dependencies
+// Note The Open API schema depends on an external library - cloud-resource-schema, so need to unzip it first.
+swaggerSources.server.code.dependsOn tasks.unzipCloudResourceSchema


what about the swagger client tasks?

This took longer to investigate and fix than I want to admit, but you're right that swagger client tasks didn't depend on this. Should be fixed now

jaycarlton · 2021-02-11T21:43:01Z

src/main/java/bio/terra/workspace/app/controller/GoogleReferenceController.java

+    GoogleBucketReference response =
+        new GoogleBucketReference()
+            .bucket(
+                ((bio.terra.workspace.service.datareference.model.GoogleBucketReference)


It's easy to set up a global prefix in Swagger.

jaycarlton · 2021-02-11T21:47:02Z

src/main/java/bio/terra/workspace/app/controller/GoogleReferenceController.java

+
+  @Override
+  public ResponseEntity<Void> updateGoogleBucketReference(
+      UUID id, UUID referenceId, UpdateDataReferenceRequestBody body) {


this method can just be updateBucketReference because it's on the Google controller.

jaycarlton · 2021-02-11T22:19:16Z

src/test/java/bio/terra/workspace/service/datareference/DataReferenceValidationUtilsTest.java

+  public void testInvalidCharInBucketName() {
+    assertThrows(
+        InvalidDataReferenceException.class,
+        () -> DataReferenceValidationUtils.validateBucketName("INVALIDBUCKETNAME"));


It would be nice to have this validation at a higher level so controlled resources can use it too.

This is in a validation util class called by the controller, I think it's actually reasonable for the various controlled resource controllers to use this bean as well

ddietterich · 2021-02-12T18:26:39Z

build.gradle

+	from zipTree(configurations.cloudResourceSchema.singleFile).matching {
+		include 'cloud_resources_uid.yaml'
+	}
+	into "${resourceDir}/api/common"


We can leave it for now. I'll make a ticket.
Another approach might be to move all of the parts into a build subdirectory and do the codegen from there.
I'll make a ticket.

zloery · 2021-02-12T18:52:24Z

Published version 0.12.0-SNAPSHOT from this.

zloery and others added 9 commits January 27, 2021 14:28

initial work, still broken

9773494

fix name and run spotless

f8e7cd0

progress

ea5233b

add basic reference validation. still needs testing and cleanup

de2f9ba

use shared UIDs for cloud resources, tests

8e912c0

move controller translation to util, rename createCow methods

2a1ef38

merge from dev

0d74bc6

formatting

0226124

update other crl versions

9235d98

jaycarlton reviewed Feb 1, 2021

View reviewed changes

zloery added 2 commits February 2, 2021 12:38

review comments

9bdadf3

add logging for invalid references

fc7d63a

zloery changed the title ~~DRAFT [PF-308] Add GCS bucket and bigquery data references~~ [PF-308] Add GCS bucket and bigquery data references Feb 2, 2021

zloery marked this pull request as ready for review February 2, 2021 18:06

zloery requested review from davidangb, ddietterich and jaycarlton February 2, 2021 18:15

jaycarlton approved these changes Feb 2, 2021

View reviewed changes

davidangb approved these changes Feb 2, 2021

View reviewed changes

zloery added 2 commits February 2, 2021 17:31

remove commented out yaml

950fdfd

merge dev changes

4a2f918

ddietterich suggested changes Feb 2, 2021

View reviewed changes

zloery added 2 commits February 3, 2021 15:47

add bucket/bq name validation, return values in errors

2e5ac95

remove typeMap

a2f3c49

merge dev changes

c2a9dd9

zloery requested a review from ddietterich February 4, 2021 18:13

zloery marked this pull request as draft February 4, 2021 19:26

zloery added 3 commits February 8, 2021 14:21

major refactoring of endpoints to per-reference type

b22c01a

add controller annotations

eaae8be

updated endpoint locations to remove ambiguity

6b17d85

zloery marked this pull request as ready for review February 8, 2021 20:22

zloery requested review from davidangb and jaycarlton February 8, 2021 20:22

make endpoints plural

84afee2

ddietterich reviewed Feb 9, 2021

View reviewed changes

davidangb reviewed Feb 10, 2021

View reviewed changes

zloery added 4 commits February 10, 2021 13:09

remove per-type delete endpoints

f792853

merge changes from dev

5ebcc69

factor out common ReferenceController, proper description wiring

26bfe9a

logging formatting

c40e94a

zloery added 2 commits February 11, 2021 10:57

formatting, fix swagger-ui interaction with remote yaml

7937bf1

merge dev changes

51c06c0

zloery requested a review from ddietterich February 11, 2021 16:55

ddietterich reviewed Feb 11, 2021

View reviewed changes

jaycarlton approved these changes Feb 11, 2021

View reviewed changes

bump version, responses to comments

ac1f476

ddietterich approved these changes Feb 12, 2021

View reviewed changes

zloery merged commit d3fa9a8 into dev Feb 12, 2021

zloery deleted the pf308 branch February 12, 2021 19:05

Conversation

zloery commented Feb 1, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jaycarlton left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

jaycarlton left a comment

Choose a reason for hiding this comment

Uh oh!

davidangb commented Feb 2, 2021

Uh oh!

davidangb left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ddietterich left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zloery commented Feb 1, 2021 •

edited

Loading

zloery commented Feb 3, 2021 •

edited

Loading