Skip to content
Merged
Show file tree
Hide file tree
Changes from 58 commits
Commits
Show all changes
104 commits
Select commit Hold shift + click to select a range
9d64ae1
Add service-to-service Node.js sample
Apr 3, 2020
64ab35f
Add service-to-service Node.js sample
kelsk Apr 3, 2020
00451bf
Update run/authentication/auth.js
kelsk Apr 3, 2020
86b0635
Update sample to address suggested changes
kelsk Apr 3, 2020
5132e26
Merge branch 'master' into auth
kelsk Apr 3, 2020
a176376
Merge pull request #1 from kelsk/auth
kelsk Apr 3, 2020
70ce8f7
Merge branch 'master' into master
kelsk Apr 3, 2020
5975b57
Initial code added
kelsk Apr 6, 2020
8e9ef37
Updated package.json; resolved issues in auth.js from review
kelsk Apr 13, 2020
867eb65
Removed auth from branch
kelsk Apr 13, 2020
62fd173
Markdown renderer & editor added
kelsk Apr 20, 2020
02dccb6
Markdown renderer & editor working
kelsk Apr 21, 2020
9ef850f
Added xss sanitizer
kelsk Apr 21, 2020
a56dca4
General cleanup
kelsk Apr 21, 2020
0bafa82
Delete .gitignore
kelsk Apr 21, 2020
bb14175
Delete .gitignore
kelsk Apr 21, 2020
1905b18
Removed errant console.log
kelsk Apr 21, 2020
8353e52
Update markdown converter, general code cleanup
kelsk Apr 23, 2020
039663d
updated README
kelsk Apr 24, 2020
39dfd1f
tests: added unit tests for editor & renderer
kelsk Apr 24, 2020
5985740
added timeout
kelsk Apr 24, 2020
0d0e2ef
tests: added editor & editor/render tests
kelsk Apr 27, 2020
7f32b81
tests: added & modified test in package.json
kelsk Apr 27, 2020
f43a2af
README: added env var
kelsk Apr 27, 2020
2db15ae
Revert "README: added env var"
kelsk Apr 27, 2020
7706dd1
README: added env var
kelsk Apr 27, 2020
0cdf727
updated author email
kelsk Apr 28, 2020
1d17988
Update run/markdown-preview/README.md
kelsk Apr 28, 2020
5fb25b1
add license to Dockerfile
kelsk Apr 29, 2020
43bb924
fixed json parsing error
kelsk Apr 29, 2020
bea221b
remove dependencies from root package.json
kelsk Apr 29, 2020
5277daf
remove xss (not needed)
kelsk Apr 30, 2020
a359786
update node version in Dockerfile
kelsk Apr 30, 2020
4391a1b
refactored code to reflect code review
kelsk Apr 30, 2020
d2410d8
updated tests to respond to new code
kelsk Apr 30, 2020
f2fe58e
update packages.json
kelsk Apr 30, 2020
54649d6
update README: Merge branch 'markdown' of https://github.com/kelsk/no…
kelsk Apr 30, 2020
88dd67d
run/markdown-preview: add sample
kelsk Apr 30, 2020
d70d1a6
update: syntax & grammar fixes
kelsk Apr 30, 2020
a8952fa
change: throw error if markdown data absent from req
kelsk Apr 30, 2020
f2706f0
change req.body validation
kelsk May 1, 2020
bca1bb3
fix syntax error
kelsk May 1, 2020
4890729
change: fixed file name
kelsk May 2, 2020
5de6e9c
syntax: remove newline
kelsk May 5, 2020
c97f02b
update: engine version
kelsk May 5, 2020
6c0897c
syntax: general fixes
kelsk May 5, 2020
8842611
code: replace json with plain text
kelsk May 6, 2020
b81c140
test: modified tests for accuracy
kelsk May 6, 2020
5a1d84e
syntax: minor fixes
kelsk May 6, 2020
971bff9
test: modified for new code
kelsk May 6, 2020
3c4ed75
added comment about xss
kelsk May 6, 2020
f4d8a99
code: unwrapped body.data
kelsk May 7, 2020
330206e
test: updated tests per code review
kelsk May 7, 2020
9f23945
code: added timeout to got request
kelsk May 8, 2020
8a0aa37
test: updated with sinon & error message reading
kelsk May 8, 2020
6bdf2b6
test: updated with sinon to read console log
kelsk May 8, 2020
42c6eb8
reformatted markdown declaration
kelsk May 8, 2020
675b19f
test: refactor to make valid tests
kelsk May 8, 2020
3503649
Updated sample to use google-auth-library
kelsk May 19, 2020
0bbd133
updated req.body conditional & markdown-it comment
kelsk May 19, 2020
d5fff16
Removed init(); added Renderer url to /render
kelsk May 19, 2020
5a7a2ec
Updated renderRequest client
kelsk May 19, 2020
f369ccc
Updated get to be async
kelsk May 19, 2020
394dc63
Updated tests to reflect new code
kelsk May 19, 2020
618b88e
Removed erroneous comment & outdated env var
kelsk May 19, 2020
a7bfaae
Removed unused js package
kelsk May 19, 2020
2b2844c
Added check for service url
kelsk May 19, 2020
eb6bbf1
Updated service url in test/render
kelsk May 19, 2020
a2ca8b5
Updated tests with urls & removed incorrect assertion
kelsk May 19, 2020
13fb1a0
updated POST request test
kelsk May 19, 2020
3084e6f
updated POST request test
kelsk May 19, 2020
829039b
removed 'then' from renderer test
kelsk May 19, 2020
b7a7eaa
test modifications
kelsk May 19, 2020
0a6e84c
Reconfigured serviceUrl
kelsk May 19, 2020
719b304
Removed sinon
kelsk May 21, 2020
5cfb4e6
Updated region tags
kelsk May 21, 2020
59e9e13
Updated valid request test
kelsk May 21, 2020
77d997c
Update: node 10 => node 12
kelsk May 27, 2020
9f9e9bd
Added end to end tests
kelsk Jun 4, 2020
258b7f2
Update run/markdown-preview/editor/test/system.test.js
kelsk Jun 4, 2020
ef89135
Merge branch 'master' into markdown
kelsk Jun 4, 2020
4fbb5f4
Added test to system.test.js
kelsk Jun 4, 2020
b071dfb
Fixed npm test
kelsk Jun 4, 2020
965b1b6
Merge branch 'markdown' of https://github.com/kelsk/nodejs-docs-sampl…
kelsk Jun 4, 2020
6bfc88b
Fixed renderer test
kelsk Jun 4, 2020
0d2d33a
Updated shell permissions
kelsk Jun 4, 2020
4658d62
Updated cleanup func
kelsk Jun 8, 2020
f449c88
Merge branch 'master' into markdown
kelsk Jun 8, 2020
9cc0974
Update var exports
kelsk Jun 8, 2020
f8483e2
Merge branch 'markdown' of https://github.com/kelsk/nodejs-docs-sampl…
kelsk Jun 8, 2020
ef43218
Added service-name.sh
kelsk Jun 9, 2020
ce5ff21
Merge branch 'master' into markdown
averikitsch Jun 9, 2020
0806459
Removed service-name.sh
kelsk Jun 9, 2020
e3b2703
Merge branch 'markdown' of https://github.com/kelsk/nodejs-docs-sampl…
kelsk Jun 9, 2020
bc1b999
Move service.sh functions into runner.sh
kelsk Jun 10, 2020
1a174f0
Merge branch 'master' into markdown
kelsk Jun 10, 2020
71eafed
Added check for BASE_URL
kelsk Jun 10, 2020
45e4ca4
Merge branch 'markdown' of https://github.com/kelsk/nodejs-docs-sampl…
kelsk Jun 10, 2020
9ed8ef5
Fixed system.test env
kelsk Jun 10, 2020
44af029
Add check for BASE_URL var
kelsk Jun 11, 2020
c4a152f
Added id token
kelsk Jun 12, 2020
35e5795
Merge branch 'master' into markdown
kelsk Jun 12, 2020
57aa5ee
Fixed test; removed allow-auth flag
kelsk Jun 12, 2020
9d2028a
Merge branch 'markdown' of https://github.com/kelsk/nodejs-docs-sampl…
kelsk Jun 12, 2020
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .kokoro/run/markdown-preview-editor.cfg
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# Format: //devtools/kokoro/config/proto/build.proto

# Set the folder in which the tests are run
env_vars: {
key: "PROJECT"
value: "run/markdown-preview/editor"
}
7 changes: 7 additions & 0 deletions .kokoro/run/markdown-preview-renderer.cfg
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# Format: //devtools/kokoro/config/proto/build.proto

# Set the folder in which the tests are run
env_vars: {
key: "PROJECT"
value: "run/markdown-preview/renderer"
}
24 changes: 24 additions & 0 deletions run/markdown-preview/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
# Cloud Run Markdown Preview Sample

[Securing Cloud Run services tutorial](https://cloud.google.com/run/docs/tutorials/secure-services) walks through how to create a secure two-service application running on Cloud Run. This application is a Markdown editor which includes a public "frontend" service which anyone can use to compose Markdown text and a private "backend" service which renders Markdown text to HTML.

For more details on how to work with this sample read the [Google Cloud Run Node.js Samples README](https://github.com/GoogleCloudPlatform/nodejs-docs-samples/tree/master/run).

## Environment Variables

Cloud Run services can be [configured with Environment Variables](https://cloud.google.com/run/docs/configuring/environment-variables).
Required variables for this sample include:

* `EDITOR_UPSTREAM_RENDER_URL`: The URL of the restricted Cloud Run service that
renders Markdown to HTML.

* `EDITOR_UPSTREAM_UNAUTHENTICATED`: (Optional) A boolean that indicates whether the render service requires an authenticated request.

## Dependencies

* **express**: Web server framework.
* **gcp-metadata**: Access Google Cloud Platform metadata server.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note if we drop use of gcp-metadata library, README will also need updating.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

* **got**: Node.js library for HTTP requests.
* **handlebars** JavaScript template engine.
* **markdown-it**: JavaScript library for parsing and rendering Markdown text.
* **xss**: Node.js HTML sanitizer.
34 changes: 34 additions & 0 deletions run/markdown-preview/editor/Dockerfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
# Copyright 2020 Google LLC
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# https://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

# Use the official lightweight Node.js 10 image.
# https://hub.docker.com/_/node
FROM node:10-slim
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Node 10 is already in maintenance mode. Can we use Node 12?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gotcha...done


# Create and change to the app directory.
WORKDIR /usr/src/app

# Copy application dependency manifests to the container image.
# A wildcard is used to ensure both package.json AND package-lock.json are copied.
# Copying this separately prevents re-running npm install on every code change.
COPY package*.json ./

# Install production dependencies.
RUN npm install --only=production

# Copy local code to the container image.
COPY . ./

# Run the web service on container startup.
CMD [ "npm", "start" ]
81 changes: 81 additions & 0 deletions run/markdown-preview/editor/main.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
// Copyright 2020 Google LLC
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// https://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

const express = require('express');
const handlebars = require('handlebars');
const { readFileSync } = require('fs');
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note as of the latest version of Node v10, we can use require('fs').promises to get a promise-returning version of the fs library. That allows using await readFile which is better than using a Sync function. The difference: sync function blocks the event loop so stuff other than your following lines of code is also blocked.

(I know you probably copied this from some code I put together, I should update!)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!

const renderRequest = require('./render.js');
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One of the reasons why the golang code was structured the way it is: I was hiding from the main flow of the program that the rendering process was an external API call. In fact, from the perspective of two of the files, I could re-implement the rendering to be wholly local and they wouldn't be able to tell.

We don't need to do that in node.js, but it's worth considering how that affects the shape of the code if you aren't trying to abstract things.

For example, should the functions in render.js be included directly in index.js? It's often better to isolate each integration into it's own "space", whether that's called a class, module, file, or something else.


const app = express();
app.use(express.json());

let url, isAuthenticated, markdownDefault, compiledTemplate, template;

const init = () => {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know we've gone a couple rounds on this, however I just wanted to flag that configuration loading is often considered it's own dedicated thing. In a larger service, I wouldn't want to include all of this in the same file:

  • Create a server
  • Load configuration
  • Define the routes/controllers

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In light of no longer needing the EDITOR_UPSTREAM_UNAUTHENTICATED env var with the new auth ID token method, I agree that the init() function should be removed entirely. The EDITOR_UPSTREAM_RENDER_URL is used only in /editor/render, so I'm going to put the logic to check for and use that env var in /editor/render. (Let me know if that's not the right idea.)

url = process.env.EDITOR_UPSTREAM_RENDER_URL;
if (!url) throw Error ("No configuration for upstream render service: add EDITOR_UPSTREAM_RENDER_URL environment variable");
isAuthenticated = !process.env.EDITOR_UPSTREAM_UNAUTHENTICATED;
if (!isAuthenticated) console.log("Editor: starting in unauthenticated upstream mode");
return {url, isAuthenticated};
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The main reason I've been using the pattern of EDITOR_UPSTREAM_UNAUTHENTICATED is that the code using the metadata server only works on Google Cloud compute platforms. However, with the new ID token helper methods rolling out, we could use the library to 1) Check if we have credentials/identity and 2) use the helper methods to mint a token locally. The upstream service can choose whether or not to verify.

If we take that approach, we should remove the EDITOR_UPSTREAM_UNAUTHENTICATED environment variable, and at that point I think it's worth reconsidering having this init() method and passing around a service object. Instead, it might be worth taking the approach of checking for process.env.EDITOR_UPSTREAM_RENDER_URL in global context and simply accessing it from process.env when the URL is needed.

This has at least some ramifications for all language variants of the sample, so raising this here and we'll update the design doc with conclusions.

@averikitsch @dinagraves @kelsk

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, essentially we wouldn't need any of these extra functions and could authorize our http request from the /render ?

Current implementation of new_request

Current implementation of /render

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the simplification, since it is hard to communicate the local issues with the metadata server. For Java, the request would simplify to this example.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice. I wonder if we'd want to make a point of having the snippet use the HTTP client built-in to the library, and have the tutorial use a popular HTTP client library except for minting the token? That allows us to keep token minting snippets as close to the IAP samples as possible, but still show a library many developers may want to use directly.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would like to show that if we can communicate it well. It would be nice for devs that just want to add a token to the request lib they are already using than have to reimplement with google libs.

};

const service = init();

// Load the template files and serve them with the Editor service.
const buildTemplate = () => {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like how this function is set up to always act to reload the template. Nice and predictable, and would help if a future evolution of this service wanted to be able to do something unusual, like dynamic reloading of templates.

try {
markdownDefault = readFileSync(__dirname + '/templates/markdown.md');
const indexTemplate = handlebars.compile(readFileSync(__dirname + '/templates/index.html', 'utf8'));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the compiled template.

compiledTemplate = indexTemplate({default: markdownDefault});
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the rendered HTML page. This should be done in the controller or otherwise outside of ain-memory caching.

return compiledTemplate;
} catch(err) {
throw Error ('Error loading template: ', err);
}
};

app.get('/', (req, res) => {
try {
template = buildTemplate();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because the file load and compilation of the template has a good amount of overhead, we only want to compile indexTemplate once, load markdownDefault once, but do want to render the template on demand. It's a bit contrived since we don't have a parameter that varies by request: maybe adding one would be useful? In the meantime to illustrate what I mean:

Suggested change
template = buildTemplate();
if (!template) template = buildTemplate();

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Understood, and updated. Using 'template' as the conditional variable to determine whether to run buildTemplate() makes the most sense to me, as opposed to creating an arbitrary parameter to determine whether the template should be rendered again.

res.status(200).send(template);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noted in comments on buildTemplate() that this is not currently the template, but the rendered HTML. We wouldn't actually return the template. Reinforcing it in both places for clarity.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updating indexTemplate, compiledTemplate, template, and buildTemplate() to the following variable/function names:
indexTemplate => compiledTemplate
compiledTemplate => renderedHtml
template => renderedHtml
buildTemplate() => buildRenderedHtml()

} catch (err) {
console.log('Error loading the Editor service: ', err);
res.status(500).send(err);
}
});

// The renderRequest makes a request to the Renderer service.
// The request returns the Markdown text converted to HTML.
app.post('/render', async (req, res) => {
try {
const markdown = req.body.data;
const response = await renderRequest(service, markdown);
res.status(200).send(response);
} catch (err) {
console.log('Error querying the Renderer service: ', err);
res.status(500).send(err);
}
});

const PORT = process.env.PORT || 8080;

app.listen(PORT, err => {
console.log(`Editor service is listening on port ${PORT}`);
});

// Exports for testing purposes.
module.exports = {
app,
init,
buildTemplate
};
31 changes: 31 additions & 0 deletions run/markdown-preview/editor/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
{
"name": "markdown-preview",
"description": "Cloud Run service to demonstrate service-to-service authentication, paired with Renderer service.",
"version": "0.0.1",
"private": true,
"license": "Apache-2.0",
"author": "Google LLC",
"repository": {
"type": "git",
"url": "https://github.com/GoogleCloudPlatform/nodejs-docs-samples.git"
},
"engines": {
"node": ">= 10.0.0"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we use Node 12?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

},
"main": "main.js",
"scripts": {
"start": "node main.js",
"test": "mocha test/*.test.js --exit"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

npm test will also try to run the test/system.test.js

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

},
"dependencies": {
"express": "^4.17.1",
"gcp-metadata": "^4.0.0",
"got": "^10.7.0",
"handlebars": "^4.7.6"
},
"devDependencies": {
"mocha": "^7.1.1",
"sinon": "^9.0.2",
"supertest": "^4.0.2"
}
}
58 changes: 58 additions & 0 deletions run/markdown-preview/editor/render.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
// Copyright 2020 Google LLC
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// https://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

const gcpMetadata = require('gcp-metadata')
const got = require('got');

// renderRequest creates a new HTTP request with IAM ID Token credential.
// This token is automatically handled by private Cloud Run (fully managed) and Cloud Functions.
const renderRequest = async (service, markdown) => {
// [START run_secure_request]
let token;

// Build the request to the Renderer receiving service.
const serviceRequestOptions = {
method: 'POST',
headers: {
'Content-Type': 'text/plain'
},
body: markdown,
timeout: 3000
};

if (service.isAuthenticated) {
try {
// Query the token with ?audience as the service URL.
const metadataServerTokenPath = `service-accounts/default/identity?audience=${service.url}`;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

idtokens-cloudrun shows the new way to mint an ID token. We should use that instead of directly interacting with the metadata server. A few considerations:

  • We should reuse the client, but attempt to mint the token for every outgoing request. The library handles caching and addresses token expiration.
  • While the library includes an HTTP client, I think there's value in having the tutorial just extract the token and use with the HTTP client library of our choice.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, with the following aspects:

  • I declared the client outside of the renderRequest function, and assign it inside the function only if it is undefined (if (!client))
  • I've used the Google auth library's getRequestHeaders() to retrieve the header from the client. This includes an ID token, which is appended to the serviceRequestHeaders in order to make an HTTP request with got.

// Fetch the token and then add it to the request header.
token = await gcpMetadata.instance(metadataServerTokenPath);
serviceRequestOptions.headers['Authorization'] = 'bearer ' + token;
} catch(err) {
throw Error('Metadata server could not respond to request: ', err);
}
};
// [END run_secure_request]

// [START run_secure_request_do]
try {
// serviceRequest converts the Markdown plaintext to HTML.
const serviceResponse = await got(service.url, serviceRequestOptions);
return serviceResponse.body;
} catch (err) {
throw Error('Renderer service could not respond to request: ', err);
};
// [END run_secure_request_do]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This region tag should probably go around where you use this function.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the tag is in the correct place corresponding to its place in the Go code, but I could use clarification if that's not the case.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

run_secure_request wraps how to make the token request. run_secure_request_do is utilizing that function.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't the 'got' request to the Renderer service the thing that is utilizing that function? As opposed to the Renderer service itself, which isn't using the token but runs as a result of the token being used?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that you don't need to implement the _do region tag unless you need to split the creation of the HTTP request from sending that request. In the case of Go this makes sense AND makes it unit testable. However, a different division may make more sense:

For example, just use run_secure_request and in that have one function to mint the token and another to use it and send an HTTP request.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kelsk region tags need to be updated

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated - the token minting and HTTP request are in separate blocks, but within the single region tag run_secure_request.

};

module.exports = renderRequest;
110 changes: 110 additions & 0 deletions run/markdown-preview/editor/templates/index.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
<!--
Copyright 2020 Google LLC

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
-->
<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="utf-8">
<meta name="viewport" content="width=device-width,initial-scale=1" />
<title>Markdown Editor</title>
<link rel="icon" type="image/png" href="data:image/png;base64,iVBORw0KGgo=">
<link href="https://unpkg.com/material-components-web@latest/dist/material-components-web.min.css" rel="stylesheet">
<script src="https://unpkg.com/material-components-web@latest/dist/material-components-web.min.js"></script>
<link rel="stylesheet" href="https://fonts.googleapis.com/icon?family=Material+Icons">
</head>
<body class="mdc-typography">

<header class="mdc-top-app-bar mdc-top-app-bar--fixed">
<div class="mdc-top-app-bar__row">
<section class="mdc-top-app-bar__section mdc-top-app-bar__section--align-start">
<span class="mdc-top-app-bar__title">Markdown Editor</span>
</section>
<section class="mdc-top-app-bar__section mdc-top-app-bar__section--align-end" role="toolbar">
<a href="#code" title="View the code"><i class="material-icons mdc-top-app-bar__action-item mdc-icon-button" aria-hidden="true">code</i></a>
<a href="#tutorial" title="Read the tutorial"><i class="material-icons mdc-top-app-bar__action-item mdc-icon-button" aria-hidden="true">assignment</i></a>
</section>
</div>
</header>

<div role="progressbar" class="mdc-linear-progress mdc-linear-progress--indeterminate mdc-top-app-bar--fixed-adjust" aria-label="Markdown Rendering Progress Bar" aria-valuemin="0" aria-valuemax="1" aria-valuenow="0">
<div class="mdc-linear-progress__bar mdc-linear-progress__primary-bar">
<span class="mdc-linear-progress__bar-inner"></span>
</div>
</div>

<main class="mdc-layout-grid">
<div class="mdc-layout-grid__inner">
<div class="mdc-layout-grid__cell mdc-layout-grid__cell--span-6">
<h2>Markdown Text</h2>
<section class="mdc-card mdc-card--outlined">
<div class="text-field-container">
<div class="mdc-text-field mdc-text-field--fullwidth md-text-field--no-label mdc-text-field--textarea mdc-ripple-upgraded">
<textarea id="editor" class="mdc-text-field__input" style="height: 36rem;">{{ default }}</textarea>
</div></div>

<div class="mdc-card__actions mdc-card__actions--full-bleed">
<button class="editor-button mdc-button mdc-card__action mdc-card__action--button mdc-ripple-surface">
<span class="mdc-button__label">Preview Rendered Markdown</span>
<i class="material-icons" aria-hidden="true">arrow_forward</i>
</button>
</div>
</section></div>

<div class="mdc-layout-grid__cell mdc-layout-grid__cell--span-6">
<h2>Rendered HTML</h2>
<section class="mdc-card mdc-card--outlined">
<div id="preview" style="height: 40rem; padding-left: 10px; padding-right: 10px">Tap "<strong>Preview Rendered Markdown</strong>" below the text entry to see rendered content.</div>
</section></div>
</div>
</div>
</main>

<script>
const preview = document.getElementById('preview');
const lp = new mdc.linearProgress.MDCLinearProgress(document.querySelector('.mdc-linear-progress'));
async function render(data = {}) {
const response = await fetch('/render', {
method: 'POST',
headers: {
'Content-Type': 'application/json'
},
body: JSON.stringify(data)
});

const text = await response.text();
if (!response.ok) {
console.log('error: Render Text: Received status code: ' + response.status);
}

return text;
}

function listener() {
lp.open();
render({data: document.getElementById('editor').value})
.then((result) => preview.innerHTML = result)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we mixing awai and then in one file?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@grayside thoughts on updating this?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we should fix this to use await wherever possible and backtrack to the other languages.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we should fix this to use await wherever possible and backtrack to the other languages.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we should fix this to use await wherever possible and backtrack to the other languages.

.catch((err) => {
console.log('Render Text: ' + err.message);
preview.innerHTML = '<h3><i aria-hidden="true" class="material-icons">error</i>Render Error</h3>\n<p>' + err.message + '</p>';
})
.finally(() => lp.close())
}

document.querySelector('.editor-button').addEventListener('click', listener);
window.addEventListener('load', listener);
</script>
</body>
</html>

19 changes: 19 additions & 0 deletions run/markdown-preview/editor/templates/markdown.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
# Playing with Markdown

This UI allows a user to write Markdown text and preview the rendered HTML.

You may be familiar with this workflow from sites such as Github or Wikipedia.

In practice, this web page does the following:

* On click of the *"Preview Rendered Markdown"* button, browser JavaScript
lifts the markdown text and sends it to the editor UI's public backend.
* The editor backend sends the text to a private Renderer service which
converts it to HTML.
* The HTML is injected into the web page in the right-side **Rendered HTML** area.

## Markdown Background

Markdown is a text-to-HTML conversion tool that allows you to convert plain text to valid HTML.

Read more about the [syntax on Wikipedia](https://en.wikipedia.org/wiki/Markdown).
Loading