Add Unix Stream codec, Split flow from FDK, update integration tests #150

zootalures · 2018-09-10T23:30:47Z

This makes a few changes, some of which break backward code-compatibility with the programming model

Adds new Unix-socket based HTTP stream codec
- In order to make this work I've flipped the flow of control for Codecs - the previous mode was fugly with an HTTP handler
Simplifies the testing contract to use a shared FIFO test codec rather than using stdin/stdout - codecs are no longer in scope for the Test Harness
Updated Headers to be multi-valued and to use canonical keys
Removes HTTP-specific parts of the main FDK contract (Request URL, Query Params, Method) - these will be dealt with with the HTTP gateway extension after
Removes appName from the event context - if we keep this it should be in the RuntimeContext
Added support for exposing FN_FN_ID
Extracts Fn-Flow from the core FDK as flow-runtime flow-api and flow-testing and adds appropriate interfaces for same.
- as part of this I had to add a new extension mechanism @FnFeature which loads the extension earlier than before (this also looks nicer for features IMHO)
- Updated spring to use this
- Ive also made the testing rule work with extension primarily to decouple flow testing from the core.
Replaces the previous shell-library based integration tests with a java test harness

…ew unix domain socket contract

zootalures · 2018-09-11T11:32:58Z

I'm working through the dependent PRs to make this pass but the the rough map of what has changed :

@mantree @jan-g:

UDS HTTP Stream change and Reflecting the new Domain models:

Updates to api/src/main/java/com/fnproject/fn/api/Headers.java to support multi-valued headers and Key canonicalisation - this is a pre-requisite to the new "It's HTTP" domain model
Changes to Entrypoint to invert the flow of control for codecs (and changes to existing legacy codecs for the same) : Also a change to Run to take the codec as a parameter of the entrypoint -this is necessary to remove Codec details from tests (I didn't want to spin up UDS clients in FnTestingRule tests - instead this now uses a test codec provided by the testing rule:
runtime/src/main/java/com/fnproject/fn/runtime/EntryPoint.java
runtime/src/main/java/com/fnproject/fn/runtime/HttpEventCodec.java (now deprecated)
runtime/src/main/java/com/fnproject/fn/runtime/Default Codec.java (now deprecated)
Updates to Testing rule to support new codec : (remove dependency on old HTTP codec implementation details - this also aligns the previous FnResult with OutputEvent (i.e. a Test Result is a n output event with a buffered body)
testing-junit4/src/main/java/com/fnproject/fn/testing/FnTestingRule.java
Updates to InputEvent to remove "appName" from the event - container context properties are now in the RuntimeContext
api/src/main/java/com/fnproject/fn/api/RuntimeContext.java (and anywhere that assumed an appName before)

Splitting Flow Runtime and Testing from the FDK runtime and testing:

Splitting Flow from Runtime:

The previous injection mechanism didn't let us load the function extensions early enough for flow - thought about change where configuration was initialises but realised it would be easier to use annotations to load features ; added @FnFeature to core api to discover "plugins that get injected on load" :
api/src/main/java/com/fnproject/fn/api/FnFeature.java
runtime/src/main/java/com/fnproject/fn/runtime/EntryPoint.java

And created a Flow Feature that loads the flow invoke:
flow-runtime/src/main/java/com/fnproject/fn/runtime/flow/FlowFeature.java

Also the RuntimeContext invoker extension path did not provide a means to indicate the priority of extensions - the simplest way I could see to make this possible was with "Phase" ordering that is "before call" and "after call" , where Phase.PreCall is "this might intercept the request" and Phase.Call is "this must handle the request"
api/src/main/java/com/fnproject/fn/api/FunctionInvoker.java

As a consequence of above all Flow Functions now need to import the flow runtime and add @FnFeature(FlowFeature.class)

I also made the corresponding changes to the Spring Function loader to turn this into a @FnFeature:

...tion/src/main/java/com/fnproject/springframework/function/SpringCloudFunctionFeature.java

Splitting flow from tests

This was a bit tricker as the test harness had flow baked in - I ended up creating an extension contract for the testing harness (more or less specifically taylored for flow now) :

testing-junit4/src/main/java/com/fnproject/fn/testing/FnTestingRuleFeature.java

and implememented this in

flow-testing/src/main/java/com/fnproject/fn/testing/flow/FlowTesting.java

(this class pulls out the implementation details previously in FnTestingRule)

Most of the other changes were refactoring tests around

jan-g

Posting a first pass now - more to come.

api/src/main/java/com/fnproject/fn/api/InputEvent.java

api/src/main/java/com/fnproject/fn/api/OutputEvent.java

flow-runtime/src/main/java/com/fnproject/fn/runtime/flow/FlowFeature.java

runtime/src/main/java/com/fnproject/fn/runtime/EntryPoint.java

runtime/src/main/java/com/fnproject/fn/runtime/HTTPStreamCodec.java

runtime/src/test/java/com/fnproject/fn/runtime/HTTPStreamCodecTest.java

runtime/src/main/java/com/fnproject/fn/runtime/HTTPStreamCodec.java

api/src/main/java/com/fnproject/fn/api/InputEvent.java

skinowski · 2018-09-11T21:18:37Z

runtime/src/main/java/com/fnproject/fn/runtime/HTTPStreamCodec.java

+
+
+        try {
+            channel = UnixServerSocketChannel.open();


although strictly not required (since we create new ephemeral mounts for these), it would still be good to add unlink() UDS path. In case a left over socket file is in there for whatever reason. Typically UDS servers do this and it would be beneficial with testing, etc. as well.

Fixed in new version when Stream codec is closed (which never really happens in the FDK but is useful for testing)

runtime/src/main/java/com/fnproject/fn/runtime/HttpEventCodec.java

runtime/src/main/java/com/fnproject/fn/runtime/HTTPStreamCodec.java

tcoupland · 2018-09-12T10:31:41Z

api/src/main/java/com/fnproject/fn/api/Headers.java

+ * <p>
+ * Headers are immutable
+ * <p>
+ * keys are are stored  and compared in a case-insensitive way


runtime/src/main/java/com/fnproject/fn/runtime/HTTPStreamCodec.java

zootalures · 2018-09-18T13:43:49Z

Note that I've not updated the HTTP-based examples I'll do that when I add gateway protocol support next.

tcoupland · 2018-09-18T13:48:19Z

integration-tests/src/main/java/com/fnproject/fn/integrationtest/IntegrationTestRule.java

-            pb.environment().put("no_proxy", getDockerLocalhost());
-            pb.environment().put("NO_PROXY", getDockerLocalhost());
+
+            // Sort of a hack for local mac running  with a prioxy


zootalures · 2018-09-18T14:28:19Z

closes #120

zootalures · 2018-09-18T14:30:21Z

closes #150

tcoupland · 2018-09-18T15:00:25Z

runtime/src/main/java/com/fnproject/fn/runtime/HTTPStreamCodec.java

+        }
+
+
+        // TODO these should really be mandatory


These are marked as mandatory in the spec, so could enforce.

jan-g · 2018-09-18T15:21:39Z

runtime/src/main/c/unix_socket.c

+    errno = 0;
+    int rv = socket(PF_UNIX, SOCK_STREAM, 0);
+
+    if (!rv) {


0 is a valid value.

RETURN VALUES A -1 is returned if an error occurs, otherwise the return value is a descriptor referencing the socket.

Fixe that and a couple of other typos I spotted on second pass,

runtime/src/main/c/unix_socket.c

jan-g · 2018-09-18T15:41:45Z

runtime/src/main/c/unix_socket.c

+    if (rv < 0) {
+        throwIOException(jenv, "error in select");
+        return -1;
+    } else if (rv == 0) {


I think:

if (rv == 0 || rv < 0 && errno == EINVAL) { /* timeout */ } else if (rv < 0) { ... }

jan-g

A few nits. God, C is a pain.

runtime/src/main/c/unix_socket.c

jan-g

LGTM, nice.

@tteggel

* Updating docs to reflect new CLI syntax (fnproject#138) * Updating docs to reflect new CLI syntax * Missed a few * fn-java-fdk: post-1.0.60 version bump [skip ci] * remove printenv * manual version bump * fn-java-fdk: post-1.0.62 version bump [skip ci] * Update FN container name (fnproject#139) [skip ci] Update FN container name from functions to fnserver at grabbing internal docker network IP * Updated dependency versions to reduce security problems (fnproject#143) Using snyk.io tools we found several dependencies had security vulnerabilities and could be updated. This does not remove all of them, but the remaining CVEs are in transitive dependencies brought in by 3rd party code. CVEs remain in: async-thumbnails example (low priority to fix) through - minio - wiremock fn-spring-cloud-function through - spring-cloud-function-context Additional changes were needed to adjust for API changes in updated dependencies: async-thumbnails example: - new minio client uses chunked file uploads so assertions needed changing fn-spring-cloud-function: - Several breaking API changes, and it seems now that Consumer behaviour is broken. * fn-java-fdk: post-1.0.63 version bump [skip ci] * preparations for the junit5 module - separation of the junit4 and junit5 code into different modules. - core module to share code between different TDD frameworks * cleaning of unused folders/modules * fixed deps in build-image module * fixed integration-tests examples * fixed deps in build-image module * deactivated integration-test 3 and 7 - cycle dependency to cli * fn-java-fdk: post-1.0.64 version bump [skip ci] * pom.xml files - refactoring * switched all versions to the latest available in maven central * switched all plugin-versions to the latest available in maven central * removed comment * will make the google compiler happy... * fn-java-fdk: post-1.0.65 version bump [skip ci] * Add Unix Stream codec, Split flow from FDK, update integration tests (fnproject#150) * http input codec * add jetty-core and UDS deps * PoC jetty core UDS event codec * revise FDK contract for initial relase, split flow from fn, support new unix domain socket contract * First pass of refactor * added annotations for for features to support flow * moving flow feature to runtime package * rename flow testing and add docs, fix race in Stream testing * fixing failing tests * Update to reflect FN_ID * more integration test fixing * fix up format name * first stab at home-grown UDS implementation * docker build fix * UDS rework, remove old UDS dep and test new one * adjust permissions on file on start * new integration tests * simlify build and integration tests, add tests for http-stream * fix ci * typo * make igntests standalone * itests run 1.8 * fix itest result * env for itests * add createDir * test failures in circle * fix test interference * Use standard properties in fdk tests, fix versions in itests * hmm no entropy on circle boxes :) * use consistent properties for versions everywhere * add waits for start * this time with feeling * diagnosing timeout * close input, try more aggressive testing * C errors * fixing up accept and twiddling with integration test start * set DOCKER_LOCALHOST * integration test fixing/finessing * cludge docker localhost * no_proxy messing * set fnserver IP correctly * typo * nits in C code * review nits * more C fix nits * Make mandatory headers mandatory again * minus comment * remove Content-Length from stripped input * fix call test * bump version to fix broken release [skip ci] * Initial cut at HTTP gateway prtotocol support * regen sig file * unit tests on HTTPGw Stuff * ign tests code * integration tests * update QR code demo to be HTTP Gateway friendly * tests and docs, organize inputs * Readme typos * fn-java-fdk: post-1.0.67 version bump [skip ci] * Fix typos, minor lints, and a small race condition in stream codec cleanup (fnproject#155) * Fix typos, minor lints, and a small race condition in stream codec cleanup * review feedback * debug log * add more logging * mask binary stuff in c debug code * Fix issue with uninitialised addres in accept * default REPO location in ign tests, fix race condition in Flow test tests * fix typo from fnproject#154 * bump jetty client to latest * fn-java-fdk: post-1.0.68 version bump [skip ci] * trying new delpoyment setting * fn-java-fdk: post-1.0.69 version bump [skip ci] * Try another new deployment setting (fnproject#158) * fn-java-fdk: post-1.0.70 version bump [skip ci] * make FnFeature multi-valued remove use of getAnnotationsByType (fnproject#160) * fn-java-fdk: post-1.0.71 version bump [skip ci] * Build and Init Images for substrate VM This adds a a build and init image that builds functions using substrateVM as consolidated binaries. * fn-java-fdk: post-1.0.72 version bump [skip ci] * CI/Release of substrate related images (fnproject#161) * first cut at substrate images * fix docker build * bump fdk for release * fn-java-fdk: post-1.0.74 version bump [skip ci] * Rev jackson-databind to deal with latest vulnerabilities. (fnproject#163) * Fix Jackson databind to deal with latest CVE * rev minio version used in async thumbnails test to pick up newer jetty version * fix up async thumbnails to test new minio version * fn-java-fdk: post-1.0.75 version bump [skip ci] * [164] Support http-stream protocol in substrate/native java. * This gets the local build.sh working again on the latest JDK8 * Remove comment * Include some CI fixes from @tteggel * Update file permissions on surefire CI fix script * Remove old default and http formats from boilerplate test * Add some logging to boilerplate test * Update FunctionsTest.java * Update FunctionsTest.java * Update FunctionsTest.java * Update FunctionsTest.java * Update FunctionsTest.java * Added some output to help if fn operation fails * Update IntegrationTestRule.java * Update IntegrationTestRule.java * Update IntegrationTestRule.java * Update pom.xml * tweak to surefire version and config * Add specific 8-friendly test and pom for boilerplate test. * Update FunctionsTest.java * Another attempt at logging a failed result...sigh * Rename func.yaml to prevent overwrite * Let's go again... * fn-java-fdk: post-1.0.76 version bump [skip ci] * fn-java-fdk: post-1.0.77 version bump [skip ci] * Add jdk11 build/runtime images (fnproject#169) * Add build for J11 images * Remove JDK9 VM memory options * fn-java-fdk: post-1.0.78 version bump [skip ci] * Use JRE instead of JDK for 11 runtime image, and add push of 11 images into CI release.sh * Reinstate SNAPSHOT version in init-native pom * fn-java-fdk: post-1.0.79 version bump [skip ci] * Replace java9 tests with java11 * Ooops... * Add JDK9 test back whilst CLI still supports it. * fn-java-fdk: post-1.0.80 version bump [skip ci] * Bump jackson version * fn-java-fdk: post-1.0.81 version bump [skip ci] * removed the (now) useless Docker & Format entries (fnproject#178) * fn-java-fdk: post-1.0.82 version bump [skip ci] * Native java fixes (fnproject#179) * Fixed maven surefire classloader. * Fixed SVM problem with max heap size in container. * fn-java-fdk: post-1.0.83 version bump [skip ci] * Removed old format codecs and tests * Remove Java 9 dockerfiles, update builds and tests * Update tests to java11 * Fix broken release * Bump version number past previous broken release * fn-java-fdk: post-1.0.85 version bump [skip ci] * Update HTTPGatewayFunctions.md * removed some legacy stuff (fn call & route) * A first cut at updating and simplifying the Java FDK Readme. Removed user tutorial already included in tutorials. Reorganized and simplified. More work to do reviewing each of the docs linked here. * simplifying the fdk repo readme along with mikes update * Update README.md * add generic link back to official docs * WIP fn user added to runtime image, working on adding user to the init-image * added fn user to runtime java * changes to RUN cmd syntax * trying to run fn user with correct cli flags * added user name * added gid * create app before deploy app * remove --local from create app * create app before deploy in all integ tests * deploy after context created * --create-app change to integ tests * removal of comments * fn-java-fdk: post-1.0.86 version bump [skip ci] * Update Java FDK string-reverse example * post-1.0.87 bump (manual) * fn-java-fdk: post-1.0.88 version bump [skip ci] * WIP adding logframer and tests to fdk-java, before each function invocation * WIP debugging test * Quick fix..? * checking if env variables are logged out to stdout and stderr, stdout gets redirected to stderr hence the duplication in the assertion * removing comments of test in wrong file * fn-java-fdk: post-1.0.89 version bump [skip ci] * adding explicit newline characters in print statements * editing tests * correcting \n * fn-java-fdk: post-1.0.90 version bump [skip ci] * Update pom.xml so circleci test will pass. JDK 11 not allowed currently. * fnproject/functions -> fnproject/fnserver * add fn user to jre11 runtime image * fn-java-fdk: post-1.0.91 version bump [skip ci] * fn-java-fdk: post-1.0.92 version bump [skip ci] * Re-order release artifact publishing By publishing tag to github first we ensure that no artifacts are pushed for a version which isn't tagged. Publishing to bintray last ensures that the CLI won't pick up a version which is not pushed everywhere. * Post-1.0.93 version bump [skip ci] * fn-java-fdk: post-1.0.94 version bump [skip ci] * Skip deployment of example code Do not attempt to publish artifacts for the string-reverse exapmle function * fn-java-fdk: post-1.0.95 version bump [skip ci] * Bump Jackson version * fn-java-fdk: post-1.0.96 version bump [skip ci] * Fix path of java binary in Dockerfile (fnproject#210) * fn-java-fdk: post-1.0.97 version bump [skip ci] * Updated to GraalVM 19. (fnproject#209) * fn-java-fdk: post-1.0.98 version bump [skip ci] * Add env to test server to allow setting fn limits (fnproject#214) work around changed paths in new j11 image * fn-java-fdk: post-1.0.99 version bump [skip ci] * Updated native-image to GraalVM 19.2.0. (fnproject#213) * Updated to GraalVM 19.2.0. * Fixed build-native Dockerfile. * fn-java-fdk: post-1.0.100 version bump [skip ci] * Bump Jackson version to 2.9.10 (fnproject#216) * fn-java-fdk: post-1.0.101 version bump [skip ci] * Update all names to Function Development Kit for Java per Shaun, per Legal, and for consistency. * Removed hyphen as best practice seems to be no hyphen * Updating abbrev to FDK for Java * fn-java-fdk: post-1.0.102 version bump [skip ci]

zootalures and others added 8 commits August 30, 2018 00:10

http input codec

dd87c35

add jetty-core and UDS deps

8ab3af4

PoC jetty core UDS event codec

348792e

revise FDK contract for initial relase, split flow from fn, support n…

d722ebb

…ew unix domain socket contract

First pass of refactor

08d2a4c

added annotations for for features to support flow

f483f85

moving flow feature to runtime package

e1ab94d

rename flow testing and add docs, fix race in Stream testing

fc707dd

zootalures requested review from mjg123, riconnon and jan-g September 10, 2018 23:30

fixing failing tests

fc9886d

zootalures requested review from tcoupland and gviedma-zz September 11, 2018 00:02

zootalures added 2 commits September 11, 2018 01:33

Update to reflect FN_ID

666cb3f

more integration test fixing

146e83c

fix up format name

9089035

jan-g reviewed Sep 11, 2018

View reviewed changes

skinowski reviewed Sep 11, 2018

View reviewed changes

api/src/main/java/com/fnproject/fn/api/InputEvent.java Show resolved Hide resolved

skinowski reviewed Sep 11, 2018

View reviewed changes

runtime/src/main/java/com/fnproject/fn/runtime/HttpEventCodec.java Show resolved Hide resolved

skinowski reviewed Sep 11, 2018

View reviewed changes

runtime/src/main/java/com/fnproject/fn/runtime/HTTPStreamCodec.java Outdated Show resolved Hide resolved

tcoupland reviewed Sep 12, 2018

View reviewed changes

runtime/src/main/java/com/fnproject/fn/runtime/HTTPStreamCodec.java Show resolved Hide resolved

zootalures added 4 commits September 12, 2018 18:46

first stab at home-grown UDS implementation

e2a3b90

docker build fix

0942c6a

UDS rework, remove old UDS dep and test new one

f478257

adjust permissions on file on start

f4abf59

rdallman mentioned this pull request Sep 14, 2018

http-stream format fnproject/fn#1202

Merged

zootalures added 9 commits September 16, 2018 07:57

diagnosing timeout

bf7248d

close input, try more aggressive testing

16f6802

C errors

97ffadb

fixing up accept and twiddling with integration test start

42ea541

set DOCKER_LOCALHOST

b8abc5c

integration test fixing/finessing

ce96bea

cludge docker localhost

ff729f2

no_proxy messing

e463adb

set fnserver IP correctly

e3f8d1d

zootalures changed the title ~~WIP Add Unix Stream codec, Split flow from FDK~~ Add Unix Stream codec, Split flow from FDK, update integration tests Sep 18, 2018

tcoupland reviewed Sep 18, 2018

View reviewed changes

typo

43cf9b5

tcoupland reviewed Sep 18, 2018

View reviewed changes

jan-g reviewed Sep 18, 2018

View reviewed changes

nits in C code

755c69d

jan-g reviewed Sep 18, 2018

View reviewed changes

runtime/src/main/c/unix_socket.c Show resolved Hide resolved

jan-g reviewed Sep 18, 2018

View reviewed changes

review nits

a4bf3df

jan-g suggested changes Sep 18, 2018

View reviewed changes

runtime/src/main/c/unix_socket.c Outdated Show resolved Hide resolved

runtime/src/main/c/unix_socket.c Outdated Show resolved Hide resolved

zootalures added 5 commits September 18, 2018 17:23

more C fix nits

78d745a

Make mandatory headers mandatory again

14d1b46

minus comment

e2209d7

remove Content-Length from stripped input

fef06de

fix call test

df25e51

jan-g approved these changes Sep 19, 2018

View reviewed changes

zootalures merged commit 0b7d2e6 into master Sep 19, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Unix Stream codec, Split flow from FDK, update integration tests #150

Add Unix Stream codec, Split flow from FDK, update integration tests #150

zootalures commented Sep 10, 2018 •

edited

Loading

zootalures commented Sep 11, 2018

jan-g left a comment

skinowski Sep 11, 2018

zootalures Sep 18, 2018

tcoupland Sep 12, 2018

zootalures commented Sep 18, 2018

tcoupland Sep 18, 2018

zootalures commented Sep 18, 2018

zootalures commented Sep 18, 2018

tcoupland Sep 18, 2018

jan-g Sep 18, 2018

zootalures Sep 18, 2018

zootalures Sep 18, 2018

jan-g Sep 18, 2018

jan-g left a comment

jan-g left a comment

Add Unix Stream codec, Split flow from FDK, update integration tests #150

Add Unix Stream codec, Split flow from FDK, update integration tests #150

Conversation

zootalures commented Sep 10, 2018 • edited Loading

zootalures commented Sep 11, 2018

UDS HTTP Stream change and Reflecting the new Domain models:

Splitting Flow Runtime and Testing from the FDK runtime and testing:

Splitting Flow from Runtime:

Splitting flow from tests

jan-g 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

zootalures commented Sep 18, 2018

Choose a reason for hiding this comment

zootalures commented Sep 18, 2018

zootalures commented Sep 18, 2018

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

jan-g left a comment

Choose a reason for hiding this comment

jan-g left a comment

Choose a reason for hiding this comment

zootalures commented Sep 10, 2018 •

edited

Loading