From ac085fbed17b740439219d3b88820a9241818b0a Mon Sep 17 00:00:00 2001 From: Evan Bonsignori Date: Wed, 5 Feb 2025 14:33:02 -0800 Subject: [PATCH 1/3] add `httpOnly` cookies fetch for use in client & add discussion link for AI Search (#54268) --- src/events/components/dotcom-cookies.ts | 69 +++++++++++++++++++ .../components/experiments/experiment.ts | 5 +- .../experiments/useShouldShowExperiment.ts | 6 +- src/frame/middleware/api.ts | 10 +++ src/search/components/input/SearchOverlay.tsx | 16 ++++- 5 files changed, 100 insertions(+), 6 deletions(-) create mode 100644 src/events/components/dotcom-cookies.ts diff --git a/src/events/components/dotcom-cookies.ts b/src/events/components/dotcom-cookies.ts new file mode 100644 index 000000000000..4f8decc768eb --- /dev/null +++ b/src/events/components/dotcom-cookies.ts @@ -0,0 +1,69 @@ +// We cannot use Cookies.get() on the frontend for httpOnly cookies +// so we need to make a request to the server to get the cookies + +type DotcomCookies = { + dotcomUsername?: string + isStaff?: boolean +} + +let cachedCookies: DotcomCookies | null = null +let inFlightPromise: Promise | null = null +let tries = 0 + +const GET_COOKIES_ENDPOINT = '/api/cookies' +const MAX_TRIES = 3 + +// Fetches httpOnly cookies from the server and cache the result +// We use an in-flight promise to avoid duplicate requests +async function fetchCookies(): Promise { + if (cachedCookies) { + return cachedCookies + } + + // If request is already in progress, return the same promise + if (inFlightPromise) { + return inFlightPromise + } + + if (tries > MAX_TRIES) { + // In prod, fail without a serious error + console.error('Failed to fetch cookies after 3 tries') + // In dev, be loud about the issue + if (process.env.NODE_ENV === 'development') { + throw new Error('Failed to fetch cookies after 3 tries') + } + + return Promise.resolve({}) + } + + inFlightPromise = fetch(GET_COOKIES_ENDPOINT) + .then((response) => { + tries++ + if (!response.ok) { + throw new Error(`Failed to fetch cookies: ${response.statusText}`) + } + return response.json() as Promise + }) + .then((data) => { + cachedCookies = data + return data + }) + .finally(() => { + // Clear the in-flight promise regardless of success or failure + // On success, subsequent calls will return the cached value + // On failure, subsequent calls will retry the request up to MAX_TRIES times + inFlightPromise = null + }) + + return inFlightPromise +} + +export async function getIsStaff(): Promise { + const cookies = await fetchCookies() + return cookies.isStaff || false +} + +export async function getDotcomUsername(): Promise { + const cookies = await fetchCookies() + return cookies.dotcomUsername || '' +} diff --git a/src/events/components/experiments/experiment.ts b/src/events/components/experiments/experiment.ts index 595a02e7d5b9..ec8832767d42 100644 --- a/src/events/components/experiments/experiment.ts +++ b/src/events/components/experiments/experiment.ts @@ -6,13 +6,13 @@ import { getActiveExperiments, } from './experiments' import { getUserEventsId } from '../events' -import Cookies from 'src/frame/components/lib/cookies' let experimentsInitialized = false export function shouldShowExperiment( experimentKey: ExperimentNames | { key: ExperimentNames }, locale: string, + isStaff: boolean, ) { // Accept either EXPERIMENTS. or EXPERIMENTS..key if (typeof experimentKey === 'object') { @@ -25,8 +25,7 @@ export function shouldShowExperiment( if (experiment.key === experimentKey) { // If the user has staffonly cookie, and staff override is true, show the experiment if (experiment.alwaysShowForStaff) { - const staffCookie = Cookies.get('staffonly') - if (staffCookie && staffCookie.startsWith('yes')) { + if (isStaff) { console.log(`Staff cookie is set, showing '${experiment.key}' experiment`) return true } diff --git a/src/events/components/experiments/useShouldShowExperiment.ts b/src/events/components/experiments/useShouldShowExperiment.ts index bfc058346898..4e04fb3de45c 100644 --- a/src/events/components/experiments/useShouldShowExperiment.ts +++ b/src/events/components/experiments/useShouldShowExperiment.ts @@ -1,6 +1,7 @@ import { useEffect, useState } from 'react' import { shouldShowExperiment } from './experiment' import { ExperimentNames } from './experiments' +import { getIsStaff } from '../dotcom-cookies' export function useShouldShowExperiment( experimentKey: ExperimentNames | { key: ExperimentNames }, @@ -13,8 +14,9 @@ export function useShouldShowExperiment( const [showExperiment, setShowExperiment] = useState(false) useEffect(() => { - const updateShouldShow = () => { - setShowExperiment(shouldShowExperiment(experimentKey, locale)) + const updateShouldShow = async () => { + const isStaff = await getIsStaff() + setShowExperiment(shouldShowExperiment(experimentKey, locale, isStaff)) } updateShouldShow() diff --git a/src/frame/middleware/api.ts b/src/frame/middleware/api.ts index 33ff9c8bb097..0bb98146b5ca 100644 --- a/src/frame/middleware/api.ts +++ b/src/frame/middleware/api.ts @@ -56,6 +56,16 @@ if (process.env.ELASTICSEARCH_URL) { ) } +// We need access to specific httpOnly cookies set on github.com from the client +// The only way to access these on the client is to fetch them from the server +router.get('/cookies', (req, res) => { + const cookies = { + isStaff: Boolean(req.cookies?.staffonly?.startsWith('yes')) || false, + dotcomUsername: req.cookies?.dotcom_user || '', + } + return res.json(cookies) +}) + router.get('*', (req, res) => { res.status(404).json({ error: `${req.path} not found` }) }) diff --git a/src/search/components/input/SearchOverlay.tsx b/src/search/components/input/SearchOverlay.tsx index 64f4eec82059..c0deef11d0f1 100644 --- a/src/search/components/input/SearchOverlay.tsx +++ b/src/search/components/input/SearchOverlay.tsx @@ -38,6 +38,7 @@ import { AutocompleteSearchHit } from '@/search/types' import { useAISearchAutocomplete } from '@/search/components/hooks/useAISearchAutocomplete' import { AskAIResults } from './AskAIResults' import { uuidv4 } from '@/events/components/events' +import { getIsStaff } from '@/events/components/dotcom-cookies' type Props = { searchOverlayOpen: boolean @@ -465,7 +466,20 @@ export function SearchOverlay({ backgroundColor: 'var(--overlay-bg-color)', }} /> - Give Feedback + { + if (await getIsStaff()) { + // Hubbers users use an internal discussion for feedback + window.open('https://github.com/github/docs-team/discussions/4952', '_blank') + } else { + // TODO: On ship date set this value + // window.open('TODO', '_blank') + } + }} + as="button" + > + {t('search.overlay.give_feedback')} + From 671ba2c542593fee11b14f73582a7822c64a13ef Mon Sep 17 00:00:00 2001 From: Rachael Sewell Date: Wed, 5 Feb 2025 14:38:22 -0800 Subject: [PATCH 2/3] Add staging moda envs (#54250) --- .../default/deployments/webapp.yaml | 57 +++++++++++++++++++ .../kubernetes/default/services/webapp.yaml | 19 +++++++ .../production/deployments/webapp.yaml | 2 +- config/moda/configuration/default/env.yaml | 9 +++ config/moda/configuration/production/env.yaml | 6 +- config/moda/deployment.yaml | 21 +++++++ 6 files changed, 110 insertions(+), 4 deletions(-) create mode 100644 config/kubernetes/default/deployments/webapp.yaml create mode 100644 config/kubernetes/default/services/webapp.yaml create mode 100644 config/moda/configuration/default/env.yaml diff --git a/config/kubernetes/default/deployments/webapp.yaml b/config/kubernetes/default/deployments/webapp.yaml new file mode 100644 index 000000000000..4c822867f693 --- /dev/null +++ b/config/kubernetes/default/deployments/webapp.yaml @@ -0,0 +1,57 @@ +apiVersion: apps/v1 +kind: Deployment +metadata: + name: webapp +spec: + replicas: 2 + selector: + matchLabels: + app: webapp + template: + metadata: + labels: + app: webapp + annotations: + # Our internal logs aren't structured so we use logfmt_sloppy to just log stdout and error + # See https://thehub.github.com/epd/engineering/dev-practicals/observability/logging/ for more details + fluentbit.io/parser: logfmt_sloppy + observability.github.com/splunk_index: docs-internal + spec: + dnsPolicy: Default + containers: + - name: webapp + image: docs-internal + resources: + requests: + cpu: 1000m + memory: 4500Mi + limits: + cpu: 8000m + memory: 16Gi + ports: + - name: http + containerPort: 4000 + protocol: TCP + envFrom: + - secretRef: + name: vault-secrets + - configMapRef: + name: kube-cluster-metadata + # application-config is created at deploy time from + # configuration set in config/moda/configuration/*/env.yaml + - configMapRef: + name: application-config + # Zero-downtime deploys + # https://thehub.github.com/engineering/products-and-services/internal/moda/feature-documentation/pod-lifecycle/#required-prestop-hook + # https://kubernetes.io/docs/concepts/containers/container-lifecycle-hooks/#container-hooks + lifecycle: + preStop: + exec: + command: ['sleep', '5'] + readinessProbe: + initialDelaySeconds: 5 + httpGet: + # WARNING: This should be updated to a meaningful endpoint for your application which will return a 200 once the app is fully started. + # See: https://kubernetes.io/docs/tasks/configure-pod-container/configure-liveness-readiness-startup-probes/#define-readiness-probes + path: /healthcheck + port: http diff --git a/config/kubernetes/default/services/webapp.yaml b/config/kubernetes/default/services/webapp.yaml new file mode 100644 index 000000000000..d504fd7d9f50 --- /dev/null +++ b/config/kubernetes/default/services/webapp.yaml @@ -0,0 +1,19 @@ +apiVersion: v1 +kind: Service +metadata: + name: webapp + labels: + service: webapp + annotations: + moda.github.net/domain-name: 'docs-internal-%environment%.service.%region%.github.net' + # HTTP app reachable inside GitHub's network (employee website) + moda.github.net/load-balancer-type: internal-http +spec: + ports: + - name: http + port: 4000 + protocol: TCP + targetPort: http + selector: + app: webapp + type: LoadBalancer diff --git a/config/kubernetes/production/deployments/webapp.yaml b/config/kubernetes/production/deployments/webapp.yaml index a487c29375e6..4c822867f693 100644 --- a/config/kubernetes/production/deployments/webapp.yaml +++ b/config/kubernetes/production/deployments/webapp.yaml @@ -37,7 +37,7 @@ spec: name: vault-secrets - configMapRef: name: kube-cluster-metadata - # application-config is crated at deploy time from + # application-config is created at deploy time from # configuration set in config/moda/configuration/*/env.yaml - configMapRef: name: application-config diff --git a/config/moda/configuration/default/env.yaml b/config/moda/configuration/default/env.yaml new file mode 100644 index 000000000000..064d730f1a94 --- /dev/null +++ b/config/moda/configuration/default/env.yaml @@ -0,0 +1,9 @@ +data: + MODA_APP_NAME: docs-internal + NODE_ENV: production + NODE_OPTIONS: '--max-old-space-size=4096' + PORT: '4000' + ENABLED_LANGUAGES: 'en,zh,es,pt,ru,ja,fr,de,ko' + RATE_LIMIT_MAX: '21' + # Moda uses a non-default port for sending datadog metrics + DD_DOGSTATSD_PORT: '28125' diff --git a/config/moda/configuration/production/env.yaml b/config/moda/configuration/production/env.yaml index d34633f438f3..f17b60dff4d6 100644 --- a/config/moda/configuration/production/env.yaml +++ b/config/moda/configuration/production/env.yaml @@ -1,8 +1,5 @@ data: MODA_APP_NAME: docs-internal - # Identifies the service deployment environment as production - # Equivalent to HEAVEN_DEPLOYED_ENV === 'production' - MODA_PROD_SERVICE_ENV: 'true' NODE_ENV: production NODE_OPTIONS: '--max-old-space-size=4096' PORT: '4000' @@ -10,3 +7,6 @@ data: RATE_LIMIT_MAX: '21' # Moda uses a non-default port for sending datadog metrics DD_DOGSTATSD_PORT: '28125' + # Identifies the service deployment environment as production + # Equivalent to HEAVEN_DEPLOYED_ENV === 'production' + MODA_PROD_SERVICE_ENV: 'true' diff --git a/config/moda/deployment.yaml b/config/moda/deployment.yaml index 4e6e3bdc1d21..b7e749477d6d 100644 --- a/config/moda/deployment.yaml +++ b/config/moda/deployment.yaml @@ -7,6 +7,27 @@ environments: profile: general region: iad + - name: staging-cedar + require_pipeline: false + notify_still_locked: true # Notify last person to lock this after an hour + cluster_selector: + profile: general + region: iad + + - name: staging-pine + require_pipeline: false + notify_still_locked: true # Notify last person to lock this after an hour + cluster_selector: + profile: general + region: iad + + - name: staging-spruce + require_pipeline: false + notify_still_locked: true # Notify last person to lock this after an hour + cluster_selector: + profile: general + region: iad + required_builds: - docs-internal-moda-config-bundle / docs-internal-moda-config-bundle - docs-internal-docker-image / docs-internal-docker-image From b20385deb827744d37786ca3369a79801a9006f6 Mon Sep 17 00:00:00 2001 From: Evan Bonsignori Date: Wed, 5 Feb 2025 15:01:18 -0800 Subject: [PATCH 3/3] Remove cache from `/api/cookies` (#54270) --- src/frame/middleware/api.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/frame/middleware/api.ts b/src/frame/middleware/api.ts index 0bb98146b5ca..63ceb9fe12f7 100644 --- a/src/frame/middleware/api.ts +++ b/src/frame/middleware/api.ts @@ -9,6 +9,7 @@ import pageInfo from '@/pageinfo/middleware' import pageList from '@/pagelist/middleware' import webhooks from '@/webhooks/middleware/webhooks.js' import { ExtendedRequest } from '@/types' +import { noCacheControl } from './cache-control' const router = express.Router() @@ -59,6 +60,7 @@ if (process.env.ELASTICSEARCH_URL) { // We need access to specific httpOnly cookies set on github.com from the client // The only way to access these on the client is to fetch them from the server router.get('/cookies', (req, res) => { + noCacheControl(res) const cookies = { isStaff: Boolean(req.cookies?.staffonly?.startsWith('yes')) || false, dotcomUsername: req.cookies?.dotcom_user || '',