diff --git a/docs/log-bucket/README.md b/docs/log-bucket/README.md index 8f6f0e7..e55f58f 100644 --- a/docs/log-bucket/README.md +++ b/docs/log-bucket/README.md @@ -59,8 +59,9 @@ Shortening it discards history that cannot be recovered afterwards. ### It is also how long the addresses are kept -The delivered field set includes `c-ip`. What expires here is therefore a record of people as well -as of requests. Rainlytics counts unique visitors by hashing the viewer's address and their user +The delivered field set includes `c-ip` by default. What expires here is therefore a record of +people as well as of requests, until a site delivers +[the field set without it](../visitors/#running-without-a-visitor-count). Rainlytics counts unique visitors by hashing the viewer's address and their user agent under a salt that rotates daily, and a scheduled rollup computes that hash from the address the object already carries. The [log delivery](../log-delivery/) page covers the field set, and [Counting visitors](../visitors/) covers the hash. diff --git a/docs/log-delivery/README.md b/docs/log-delivery/README.md index a5b97d8..3eb6897 100644 --- a/docs/log-delivery/README.md +++ b/docs/log-delivery/README.md @@ -93,18 +93,23 @@ last exactly as long as the log objects do. On the defaults that is 365 days, pl superseded version survives, and the [log bucket](../log-bucket/) page has both numbers and how to change them. -A site that would rather not keep addresses can deliver everything else: +A site that would rather not keep addresses delivers the named set without it: ```typescript -import { deliveredLogFieldNames } from "@kensio/rainlytics"; +import { logFieldNamesWithoutAddress } from "@kensio/rainlytics"; new CloudFrontLogDelivery(this, "RainlyticsDelivery", { distributionId: "E1EXAMPLE1234", logBucket: logs.bucket, - fields: deliveredLogFieldNames.filter((field) => field !== "c-ip"), + fields: logFieldNamesWithoutAddress, }); ``` +That is the only line a site changes. The [log table](../log-table/) declares the columns the +delivery asks for and gets no `c_ip`. The [summary schedule](../summary-schedule/) reads the table, +computes the same five questions with the visitor count off, and is granted no permission to read +the salt. The SSM parameter a default deployment needs before its first run never has to exist. + Pageviews, referrers, devices, status codes and geography all carry on. The visitor count is the one thing that stops being computable, and no later job can recover it for the days the field was absent. Geography survives because CloudFront resolves `c-country` at the edge from an address the diff --git a/docs/log-table/README.md b/docs/log-table/README.md index 7bd2836..adee1bf 100644 --- a/docs/log-table/README.md +++ b/docs/log-table/README.md @@ -137,12 +137,16 @@ and none of that has been tested here. ## `c_ip` is the viewer's address -The table has a column for it because the delivery asks for one. Rainlytics counts unique visitors -as a daily-rotating hash of the address and the user agent, and the rollup computing that hash reads -this column. The [log delivery](../log-delivery/) page says what the field set holds, and the [log -bucket](../log-bucket/) page says for how long. +The table has a column for it where the delivery asks for one, which the default field set does. +Rainlytics counts unique visitors as a daily-rotating hash of the address and the user agent, and +the rollup computing that hash reads this column. The [log delivery](../log-delivery/) page says +what the field set holds, and the [log bucket](../log-bucket/) page says for how long. -Three things follow for anything querying this table. +A delivery configured with `logFieldNamesWithoutAddress` produces a table with no such column. A +[summary schedule](../summary-schedule/) over that table computes no visitor count, and every other +column and question is the same. + +Three things follow for anything querying a table that has the column. Records delivered before the field was added carry no address. Athena answers `null` for them and reports success, and a visitor count over those days comes back low. diff --git a/docs/visitors/README.md b/docs/visitors/README.md index e9bb8df..0a29edc 100644 --- a/docs/visitors/README.md +++ b/docs/visitors/README.md @@ -136,8 +136,8 @@ A run that meets no parameter fails and says so, naming the parameter and printi `pageviews` alone, and it is one of the five questions a deployment gets when it passes no `rollups` of its own. A default deployment therefore reads the salt parameter, and the secret has to be there -before its first run. A deployment that wants none passes `rollups` without a question that counts -visitors, and needs no parameter. +before its first run. [Running without a visitor count](#running-without-a-visitor-count) has the +deployment that reads no parameter at all. A rollup says it counts with `countsVisitors`: @@ -161,11 +161,51 @@ recomputing two windows, come to 250 queries a day and about 38 cents a month. T `pageviews` adds 50 of those, which is about 8 cents. The [summary schedule](../summary-schedule/#what-it-costs) page has the arithmetic. +## Running without a visitor count + +A site that delivers no viewer address counts no visitors, and nothing else about it changes. + +```typescript +import { logFieldNamesWithoutAddress } from "@kensio/rainlytics"; + +new CloudFrontLogDelivery(this, "RainlyticsDelivery", { + distributionId: "E1EXAMPLE1234", + logBucket: logs.bucket, + fields: logFieldNamesWithoutAddress, +}); +``` + +That is the only line a site changes. The [log table](../log-table/) describes what the delivery +writes and the [summary schedule](../summary-schedule/) reads the table. Both follow. The schedule +computes the same five questions with the count off, needs no salt parameter, and is granted no +`ssm:GetParameter`. Summaries carry no `visitors` field, which a reader tells apart from a count of +zero. + +A deployment naming its own questions says so per question: + +```typescript +import { pageviews, referrers, withoutVisitorCount } from "@kensio/rainlytics"; + +new RollupSummaries(this, "RainlyticsSummaries", { + table, + workgroup, + rollups: [withoutVisitorCount(pageviews), referrers], +}); +``` + +A question that counts visitors over a table with no address is refused at synthesis, naming the +question. Left alone it would run once an hour against a column the table has never heard of. + +The choice sits on the delivery because the delivery is what writes the raw store. Turning the +address off later leaves every address already written where it is, until the [log +bucket](../log-bucket/) expiry reaches it. + ## Where the addresses are The raw log bucket holds viewer addresses in the clear, for as long as it holds anything. That is the price [#53](https://github.com/KensioSoftware/rainlytics/issues/53) paid for a visitor count, -and the [log bucket](../log-bucket/) page has the expiry that decides how long it lasts. +and the [log bucket](../log-bucket/) page has the expiry that decides how long it lasts. A site +running the field set above has none of them to keep. The salt protects the identifier and never the source. Anyone who can read the log bucket has the addresses themselves, at better resolution than any digest would give them. diff --git a/src/cdk/computed-questions.ts b/src/cdk/computed-questions.ts new file mode 100644 index 0000000..024f01d --- /dev/null +++ b/src/cdk/computed-questions.ts @@ -0,0 +1,81 @@ +// Which questions a deployment computes, given what its table carries. +// +// Apart from `summary-configuration.ts` because it answers one question that +// nothing else in the settling has to think about. Every other choice a +// deployment makes is between what it was told and a default. This one is +// between what it was told and what the delivered field set can support. + +import { + countsVisitorsFrom, + missingVisitorCountFields, +} from "../log-fields.js"; +import { type Rollup, withoutVisitorCount } from "../rollups.js"; +import { rollups } from "../rollup-questions.js"; +import type { RollupSummariesProps } from "./summary-configuration.js"; + +/** + * The questions this deployment computes, against the table it reads. + * + * A visitor count is the one question needing a field the delivery can be + * configured without. The shipped questions therefore follow the table. A + * deployment over a table carrying no viewer address gets the same five + * questions with the count off, and needs no salt for them. + * + * A caller that asked for the count by name gets an error instead. Dropping + * what it asked for would leave a deployment computing something other than + * what its code says. + * + * @throws {Error} where a named question counts visitors the table cannot + * identify. + */ +export function computedQuestions( + props: RollupSummariesProps, +): readonly Rollup[] { + if (countsVisitorsFrom(props.table.fields)) { + return props.rollups ?? rollups; + } + + if (props.rollups === undefined) { + return rollups.map((rollup) => withoutVisitorCount(rollup)); + } + + assertNothingCountsVisitors(props.rollups, props.table.fields); + + return props.rollups; +} + +/** + * Refuses questions this table cannot answer. + * + * At synthesis, where somebody can still read it. The query would otherwise + * name a column the table has never heard of, and fail once an hour in a + * bucket nobody is watching. + * + * The message names the fields actually absent. A delivery keeping the + * address and dropping the user agent counts nobody, and being told to add + * the address would send its author looking at a field already there. + * + * @throws {Error} naming the questions, what is missing, and how to turn + * their count off. + */ +function assertNothingCountsVisitors( + asked: readonly Rollup[], + fields: readonly string[], +): void { + const counting = asked.filter((rollup) => rollup.countsVisitors === true); + + if (counting.length === 0) { + return; + } + + const named = counting.map((rollup) => rollup.name).join(", "); + const missing = missingVisitorCountFields(fields).join(" and "); + + throw new Error( + `${named} counts visitors, and this deployment's delivery leaves out` + + ` ${missing}. A visitor is a hash of the viewer's address and their` + + ` user agent, and both have to be delivered for one to be counted.` + + ` Either add ${missing} to the delivered field set, or wrap the` + + ` question in withoutVisitorCount.`, + ); +} diff --git a/src/cdk/log-table.test.ts b/src/cdk/log-table.test.ts index 9defede..9557a15 100644 --- a/src/cdk/log-table.test.ts +++ b/src/cdk/log-table.test.ts @@ -10,7 +10,10 @@ import { describe, expect, it } from "vitest"; import { deployStacks, simStartedAt } from "#test/simulated-deployment.js"; import { defaultLogDataset, qualifiedTableName } from "../dataset.js"; -import { deliveredLogColumnNames } from "../log-fields.js"; +import { + deliveredLogColumnNames, + logFieldNamesWithoutAddress, +} from "../log-fields.js"; import { partitionKeyNames, partitionLocationTemplate, @@ -401,6 +404,22 @@ describe("the Glue table over delivered logs", () => { ]); }); + it("describes no address where the delivery asked for none", async () => { + // Given a site delivering the field set that holds no personal data. + const table = catalogTable( + await deployTable({ + delivery: { fields: [...logFieldNamesWithoutAddress] }, + }), + ); + + // Then the table has no column an address could be read out of, while + // every other question still has the column it groups by. + const columns = table.columns.map((column) => column.Name); + expect(columns).not.toContain("c_ip"); + expect(columns).toContain("cs_uri_stem"); + expect(columns).toContain("cs_user_agent"); + }); + describe("what it refuses to build", () => { /** A stack holding a log bucket, a delivery and whatever a case adds. */ const synthesise = ( diff --git a/src/cdk/log-table.ts b/src/cdk/log-table.ts index bc3ced9..1be23c9 100644 --- a/src/cdk/log-table.ts +++ b/src/cdk/log-table.ts @@ -115,6 +115,16 @@ export class LogTable extends Construct { */ readonly logBucket: LogDeliveryBucket; + /** + * The fields the table describes, as the deliveries agreed on them. + * + * Here so that what reads the table can see what is in it. `RollupSummaries` + * asks whether the viewer's address is among these before it schedules a + * visitor count, because a query naming a column the table has no idea about + * fails once an hour in a bucket nobody is watching. + */ + readonly fields: readonly string[]; + constructor(scope: Construct, id: string, props: LogTableProps) { super(scope, id); @@ -134,6 +144,8 @@ export class LogTable extends Construct { const fields = deliveredLogFieldsNamed(delivery.fields); const format = logTableFormat(delivery.outputFormat, fields); + this.fields = delivery.fields; + this.database = new CfnDatabase(this, "Database", { catalogId, databaseInput: { diff --git a/src/cdk/rollup-summaries.test.ts b/src/cdk/rollup-summaries.test.ts index 2091a4e..4688654 100644 --- a/src/cdk/rollup-summaries.test.ts +++ b/src/cdk/rollup-summaries.test.ts @@ -22,11 +22,16 @@ import { describe, expect, it } from "vitest"; import { deployStacks, simStartedAt } from "#test/simulated-deployment.js"; import { summaryEnvironment } from "../functions/summary-deployment.js"; +import { logFieldNamesWithoutAddress } from "../log-fields.js"; import { partitionPrefix } from "../partitions.js"; import { pageviews } from "../rollup-questions.js"; import type { RollupSummary } from "../rollup-summaries.js"; import { summarySchemaVersion } from "../rollup-summaries.js"; -import { defaultRedirectStatuses, windowPlaceholder } from "../rollups.js"; +import { + defaultRedirectStatuses, + windowPlaceholder, + withoutVisitorCount, +} from "../rollups.js"; import { defaultVisitorSaltParameter, visitorSaltPlaceholder, @@ -51,6 +56,13 @@ describe("computing rollup summaries on a schedule", () => { const deployAnalytics = async ( over: Partial = {}, inStack: (stack: Stack) => Partial = () => ({}), + site: { + /** What the delivery asks CloudFront for, defaulting to the whole set. */ + readonly fields?: readonly string[]; + + /** Whether the account holds a visitor salt at all. */ + readonly salt?: boolean; + } = {}, ) => { const logBucketName = `rainlytics-logs-${faker.string.uuid()}`; const summariesBucketName = `rainlytics-summaries-${faker.string.uuid()}`; @@ -72,6 +84,7 @@ describe("computing rollup summaries on a schedule", () => { const delivery = new CloudFrontLogDelivery(stack, "Delivery", { distributionId: distribution.distributionId, logBucket: logs.bucket, + ...(site.fields === undefined ? {} : { fields: site.fields }), }); const table = new LogTable(stack, "RainlyticsTable", { deliveries: [delivery], @@ -97,18 +110,22 @@ describe("computing rollup summaries on a schedule", () => { // The salt secret, put where a site's operator puts it. Nothing in the // stack creates it, because CloudFormation writes no SecureString and a - // secret in a template is not one. `docs/visitors/` has the command. - await simAws - .region("us-east-1") - .account() - .ssm() - .putParameter({ - input: { - Name: defaultVisitorSaltParameter, - Type: "SecureString", - Value: faker.string.hexadecimal({ length: 64, prefix: "" }), - }, - }); + // secret in a template is not one. `docs/visitors/` has the command. A + // case about a deployment counting no visitors leaves it out, which is + // the account a site running without one actually has. + if (site.salt !== false) { + await simAws + .region("us-east-1") + .account() + .ssm() + .putParameter({ + input: { + Name: defaultVisitorSaltParameter, + Type: "SecureString", + Value: faker.string.hexadecimal({ length: 64, prefix: "" }), + }, + }); + } return { simAws, @@ -448,6 +465,29 @@ describe("computing rollup summaries on a schedule", () => { await expect(summaryAt(deployed, closedHourKey)).resolves.toBeUndefined(); }); + it("summarises an hour with no salt where the address is undelivered", async () => { + // Given a site delivering the field set that holds no personal data, in + // an account where nobody has ever created a visitor salt. + const deployed = await deployAnalytics( + { rollups: [withoutVisitorCount(pageviews)] }, + () => ({}), + { fields: logFieldNamesWithoutAddress, salt: false }, + ); + const { "c-ip": _address, ...asDelivered } = aRecord(theClosedHour); + await putDelivered(deployed, theClosedHour, [asDelivered, asDelivered]); + + // When the schedule fires. + await deployed.simAws.clock().advanceBy({ minutes: 16 }); + + // Then the hour is summarised as usual and carries no visitor count. A + // reader sees the field absent rather than a zero, and the run needed no + // parameter to get there. + const summary = await summaryAt(deployed, closedHourKey); + + expect(summary?.rows).toStrictEqual([{ path: "/", views: "2" }]); + expect(summary?.visitors).toBeUndefined(); + }); + it("hands the visitor count to the schedule without the salt", async () => { // Given a deployment of the question as Rainlytics ships it, which counts // visitors. diff --git a/src/cdk/rollup-summaries.ts b/src/cdk/rollup-summaries.ts index a59231d..84a623a 100644 --- a/src/cdk/rollup-summaries.ts +++ b/src/cdk/rollup-summaries.ts @@ -68,6 +68,7 @@ export class RollupSummaries extends Construct { workgroup: props.workgroup, bucket: this.bucket, windows: settled.windows, + countsVisitors: settled.countsVisitors, ...(props.visitorSaltParameter === undefined ? {} : { visitorSaltParameter: props.visitorSaltParameter }), diff --git a/src/cdk/summary-configuration.test.ts b/src/cdk/summary-configuration.test.ts index ba4f4d0..139d753 100644 --- a/src/cdk/summary-configuration.test.ts +++ b/src/cdk/summary-configuration.test.ts @@ -1,10 +1,13 @@ import { faker } from "@faker-js/faker"; +import type { Policy } from "aws-cdk-lib/aws-iam"; import { Bucket } from "aws-cdk-lib/aws-s3"; import type { CfnSchedule } from "aws-cdk-lib/aws-scheduler"; import { App, Stack } from "aws-cdk-lib/core"; import { describe, expect, it } from "vitest"; +import { logFieldNamesWithoutAddress } from "../log-fields.js"; import { pageviews, rollups } from "../rollup-questions.js"; +import { withoutVisitorCount } from "../rollups.js"; import { CloudFrontLogDelivery } from "./log-delivery.js"; import { LogTable } from "./log-table.js"; import { QueryWorkgroup } from "./query-workgroup.js"; @@ -20,6 +23,7 @@ import type { RollupSummariesProps } from "./summary-configuration.js"; describe("what a deployment of the summaries computes", () => { const summariesIn = ( over: Partial = {}, + fields?: readonly string[], ): RollupSummaries => { const stack = new Stack(new App(), "AnalyticsStack", { env: { account: "123456789012", region: "us-east-1" }, @@ -27,6 +31,7 @@ describe("what a deployment of the summaries computes", () => { const delivery = new CloudFrontLogDelivery(stack, "Delivery", { distributionId: "E1EXAMPLE1234", logBucket: new Bucket(stack, "Logs"), + ...(fields === undefined ? {} : { fields }), }); return new RollupSummaries(stack, "RainlyticsSummaries", { @@ -36,6 +41,30 @@ describe("what a deployment of the summaries computes", () => { }); }; + /** The actions the summary function's role was granted. */ + const grantedActions = (summaries: RollupSummaries): readonly string[] => { + const policy = summaries.lambda.role?.node.tryFindChild( + "DefaultPolicy", + ) as Policy; + const document = Stack.of(summaries).resolve(policy.document) as { + readonly Statement: readonly { + readonly Action: string | readonly string[]; + }[]; + }; + + return document.Statement.flatMap((statement) => [statement.Action].flat()); + }; + + /** The visitor SQL a schedule carries, where it carries any. */ + const visitorSqlOf = ( + schedule: CfnSchedule | undefined, + ): string | undefined => { + const target = schedule?.target as CfnSchedule.TargetProperty; + + return (JSON.parse(String(target.input)) as { visitorSql?: string }) + .visitorSql; + }; + it("computes every shipped question on both cadences", () => { // Given a site that said nothing but where its table and workgroup are. const summaries = summariesIn(); @@ -89,6 +118,98 @@ describe("what a deployment of the summaries computes", () => { ]); }); + it("counts visitors where the table carries the viewer's address", () => { + // Given a deployment over a table built from the default field set. + const summaries = summariesIn({ + rollups: [pageviews], + granularities: ["hourly"], + }); + + // Then the schedule carries a second query counting them, salted per day, + // and the job may read the salt. + expect(visitorSqlOf(summaries.schedules[0])).toContain("c_ip"); + expect(grantedActions(summaries)).toContain("ssm:GetParameter"); + }); + + it("counts no visitors where the delivery left the address out", () => { + // Given a site delivering a field set with no viewer address, which is + // the one configuration holding no personal data. + const summaries = summariesIn( + { granularities: ["hourly"] }, + logFieldNamesWithoutAddress, + ); + + // Then it still computes every shipped question, and none of them carries + // a query naming a column the table has never heard of. + expect(summaries.schedules).toHaveLength(rollups.length); + for (const schedule of summaries.schedules) { + expect(visitorSqlOf(schedule)).toBeUndefined(); + } + }); + + it("reads no salt for a deployment counting no visitors", () => { + // Given the same deployment, whose SSM parameter nobody has created. + const summaries = summariesIn( + { granularities: ["hourly"] }, + logFieldNamesWithoutAddress, + ); + + // When the function's policy is read. + const actions = grantedActions(summaries); + + // Then it was granted nothing on Systems Manager, while keeping the reads + // its queries need. A site running no count has no parameter to point + // that permission at. + expect(actions).toContain("athena:StartQueryExecution"); + expect(actions).not.toContain("ssm:GetParameter"); + }); + + it("takes a named question whose visitor count was turned off", () => { + // Given a site delivering no address and naming the one question it + // wants, with the count taken off it. + const summaries = summariesIn( + { rollups: [withoutVisitorCount(pageviews)], granularities: ["hourly"] }, + logFieldNamesWithoutAddress, + ); + + // Then it deploys, and the schedule carries the question without a count. + expect(summaries.schedules).toHaveLength(1); + expect(visitorSqlOf(summaries.schedules[0])).toBeUndefined(); + }); + + it("refuses a question that counts visitors the table cannot identify", () => { + // Given a site that left the address out of the delivery and then asked + // for the visitor count by name anyway. + const building = (): unknown => + summariesIn( + { rollups: [pageviews], granularities: ["hourly"] }, + logFieldNamesWithoutAddress, + ); + + // Then it is refused at synthesis. Dropping the count silently would run + // a deployment computing something other than what its code asked for, + // and running it would fail hourly against a missing column. + expect(building).toThrow(/counts visitors/u); + expect(building).toThrow(/c-ip/u); + }); + + it("names the identifying field a narrowed delivery actually left out", () => { + // Given a delivery keeping the address and dropping the user agent, which + // counts nobody just as surely. + const building = (): unknown => + summariesIn({ rollups: [pageviews], granularities: ["hourly"] }, [ + "timestamp(ms)", + "cs-uri-stem", + "c-ip", + ]); + + // Then the refusal names the user agent and leaves the address out of it. + // Being told to add a field already delivered sends its author looking in + // the wrong place. + expect(building).toThrow(/cs\(User-Agent\)/u); + expect(building).not.toThrow(/c-ip/u); + }); + it("refuses a deployment that would compute nothing", () => { // Given a site that passed an empty list of questions, which is not the // same as leaving the prop out. diff --git a/src/cdk/summary-configuration.ts b/src/cdk/summary-configuration.ts index 067b030..3572518 100644 --- a/src/cdk/summary-configuration.ts +++ b/src/cdk/summary-configuration.ts @@ -10,10 +10,10 @@ import type { Duration } from "aws-cdk-lib/core"; import { savedQueryPrefix } from "../dataset.js"; import type { Rollup } from "../rollups.js"; -import { rollups } from "../rollup-questions.js"; import { defaultRecomputedWindows } from "../summary-runs.js"; import type { SummaryGranularity } from "../summary-windows.js"; import { summaryGranularities } from "../summary-windows.js"; +import { computedQuestions } from "./computed-questions.js"; import type { LogTable } from "./log-table.js"; import type { QueryWorkgroup } from "./query-workgroup.js"; import { assertRequestedNames } from "./saved-query-names.js"; @@ -84,8 +84,13 @@ export interface RollupSummariesProps extends SummaryBucketProps { * * `pageviews` counts visitors, and it is one of the five questions a * deployment that passes no `rollups` gets. The parameter therefore has to - * exist before a default deployment's first run. A deployment that wants - * none passes `rollups` without a question that counts visitors. + * exist before a default deployment's first run. + * + * A deployment whose table carries no viewer address counts no visitors, + * needs no parameter here, and is granted no permission to read one. That + * follows the delivery's field set, so a site opting out passes + * `logFieldNamesWithoutAddress` to `CloudFrontLogDelivery` and changes + * nothing here. * * `docs/visitors/` has the command that makes one and why the secret * stands rather than rotating. @@ -132,6 +137,15 @@ export interface SummaryConfiguration { /** The questions to compute. */ readonly rollups: readonly Rollup[]; + /** + * Whether any of them counts visitors. + * + * False where the table carries no viewer address. The deployment then + * needs no salt parameter, and `SummaryFunction` leaves it out rather than + * granting a read on a parameter nothing will look at. + */ + readonly countsVisitors: boolean; + /** The windows to compute them over. */ readonly granularities: readonly SummaryGranularity[]; @@ -158,7 +172,7 @@ export interface SummaryConfiguration { export function summaryConfiguration( props: RollupSummariesProps, ): SummaryConfiguration { - const computing = props.rollups ?? rollups; + const computing = computedQuestions(props); const granularities = props.granularities ?? summaryGranularities; assertSomethingToCompute(computing, granularities); @@ -167,6 +181,7 @@ export function summaryConfiguration( return { rollups: computing, + countsVisitors: computing.some((rollup) => rollup.countsVisitors === true), granularities, windows: props.recomputedWindows ?? defaultRecomputedWindows, lag: props.lag ?? defaultSummaryLag, diff --git a/src/cdk/summary-function.ts b/src/cdk/summary-function.ts index b3b883c..82f26e7 100644 --- a/src/cdk/summary-function.ts +++ b/src/cdk/summary-function.ts @@ -20,13 +20,7 @@ import type { LogTable } from "./log-table.js"; import type { QueryWorkgroup } from "./query-workgroup.js"; import type { SummariesBucket } from "./summary-bucket.js"; import { summaryCodePath, summaryHandlerName } from "./summary-code.js"; -import { - athenaStatements, - catalogStatements, - logReadStatements, - resultsStatements, - visitorSaltStatements, -} from "./summary-permissions.js"; +import { summaryJobStatements } from "./summary-permissions.js"; /** What the summary function needs telling. */ export interface SummaryFunctionProps { @@ -42,6 +36,15 @@ export interface SummaryFunctionProps { /** How many closed windows one run computes. */ readonly windows: number; + /** + * Whether any of this deployment's questions counts visitors. + * + * False where the table carries no viewer address. The function is then + * given no permission to read the salt, and the parameter it names need + * never exist. + */ + readonly countsVisitors: boolean; + /** * The SSM parameter holding the visitor salt secret. * @@ -117,15 +120,13 @@ export class SummaryFunction extends Construct { }, }); - for (const statement of [ - ...athenaStatements(this, props.workgroup.workgroupName), - ...catalogStatements(this, props.table.dataset), - ...logReadStatements(props.table.logBucket, this.lambda), - // Athena writes every query's output to the workgroup's results - // location as the caller, and reads it back to answer GetQueryResults. - ...resultsStatements(props.workgroup.resultsBucket, this.lambda), - ...visitorSaltStatements(this, saltParameter), - ]) { + for (const statement of summaryJobStatements(this, { + workgroup: props.workgroup, + table: props.table, + grantee: this.lambda, + saltParameter, + countsVisitors: props.countsVisitors, + })) { this.lambda.addToRolePolicy(statement); } diff --git a/src/cdk/summary-permissions.ts b/src/cdk/summary-permissions.ts index 459b9f6..e18e257 100644 --- a/src/cdk/summary-permissions.ts +++ b/src/cdk/summary-permissions.ts @@ -19,7 +19,9 @@ import type { Construct } from "constructs"; import type { LogDataset } from "../dataset.js"; import type { LogDeliveryBucket } from "./delivery-bucket.js"; +import type { LogTable } from "./log-table.js"; import type { QueryResultsBucket } from "./query-results-bucket.js"; +import type { QueryWorkgroup } from "./query-workgroup.js"; import type { SummariesBucket } from "./summary-bucket.js"; /** @@ -131,10 +133,10 @@ export function catalogStatements( * A site naming a customer key of its own grants `kms:Decrypt` on it as well, * the way a customer-encrypted log bucket hands out its own. * - * Granted whether or not any of this deployment's questions count visitors. A - * statement naming a parameter nobody reads costs nothing, and a question - * gaining a visitor count later is then a template change rather than a run - * that fails on a permission. + * Granted to a deployment whose questions count visitors, and left off one + * whose table carries no viewer address. Nothing there can gain a visitor + * count without redelivering the field, and the parameter is then one a site + * running no count would have to create for a read that never happens. */ export function visitorSaltStatements( scope: Construct, @@ -248,3 +250,36 @@ export function summaryReadStatements( }), ]; } + +/** + * Everything the scheduled job's role is granted, in one list. + * + * Here rather than in `SummaryFunction` so that what the job may do is read + * in one place, beside the statements saying what each part of it is for. + * + * The salt is the only conditional one. A deployment whose table carries no + * viewer address counts no visitors, and granting it a read on a parameter + * nobody created would describe a permission the site has to satisfy. + */ +export function summaryJobStatements( + scope: Construct, + granted: { + readonly workgroup: QueryWorkgroup; + readonly table: LogTable; + readonly grantee: IGrantable; + readonly saltParameter: string; + readonly countsVisitors: boolean; + }, +): readonly PolicyStatement[] { + return [ + ...athenaStatements(scope, granted.workgroup.workgroupName), + ...catalogStatements(scope, granted.table.dataset), + ...logReadStatements(granted.table.logBucket, granted.grantee), + // Athena writes every query's output to the workgroup's results location + // as the caller, and reads it back to answer GetQueryResults. + ...resultsStatements(granted.workgroup.resultsBucket, granted.grantee), + ...(granted.countsVisitors + ? visitorSaltStatements(scope, granted.saltParameter) + : []), + ]; +} diff --git a/src/index.ts b/src/index.ts index 0297c11..c1ffe29 100644 --- a/src/index.ts +++ b/src/index.ts @@ -30,6 +30,7 @@ export { rowsFor, summarisedWindow, windowPlaceholder, + withoutVisitorCount, } from "./rollups.js"; export { neverComputed, @@ -94,13 +95,17 @@ export { } from "./dataset.js"; export { availableLogFields, + countsVisitorsFrom, type DeliveredLogField, deliveredLogColumnNames, deliveredLogFieldNames, deliveredLogFields, deliveredLogFieldsNamed, logColumnName, + logFieldNamesWithoutAddress, omittedLogFields, + visitorAddressField, + visitorCountFields, } from "./log-fields.js"; export { defaultFirstPartitionYear, diff --git a/src/log-fields.test.ts b/src/log-fields.test.ts index 7c82fb2..85418be 100644 --- a/src/log-fields.test.ts +++ b/src/log-fields.test.ts @@ -2,12 +2,15 @@ import { describe, expect, it } from "vitest"; import { availableLogFields, + countsVisitorsFrom, deliveredLogColumnNames, deliveredLogFields, deliveredLogFieldNames, deliveredLogFieldsNamed, logColumnName, + logFieldNamesWithoutAddress, omittedLogFields, + visitorAddressField, } from "./log-fields.js"; describe("the delivered log field set", () => { @@ -21,6 +24,24 @@ describe("the delivered log field set", () => { } }); + it("drops only the address from the set that leaves visitors uncounted", () => { + // Given the field set a site delivers to hold no personal data. + // Then it is the delivered set with the viewer's address taken out, and + // every other question reads the same columns it always did. + expect(logFieldNamesWithoutAddress).not.toContain(visitorAddressField); + expect(logFieldNamesWithoutAddress).toStrictEqual( + deliveredLogFieldNames.filter((name) => name !== visitorAddressField), + ); + }); + + it("counts visitors from the default set and from nothing narrower", () => { + // Given the two field sets a site chooses between. + // Then the default identifies a viewer and the other one cannot. This is + // what `RollupSummaries` reads to decide whether to schedule a count. + expect(countsVisitorsFrom(deliveredLogFieldNames)).toBe(true); + expect(countsVisitorsFrom(logFieldNamesWithoutAddress)).toBe(false); + }); + it("records omissions that CloudFront actually offers", () => { // Given the fields deliberately left out. // Then each is a real field, so the list documents a decision rather than diff --git a/src/log-fields.ts b/src/log-fields.ts index 63895c0..8f33440 100644 --- a/src/log-fields.ts +++ b/src/log-fields.ts @@ -163,6 +163,71 @@ export const deliveredLogFieldNames: readonly string[] = deliveredLogFields.map( (field) => field.name, ); +/** + * The field carrying the viewer's address. + * + * Named here because three places have to agree about it. A delivery decides + * whether to ask for it, a table decides whether to describe it, and + * `RollupSummaries` decides whether anything can count visitors. Spelling it + * three times is how they would come to disagree. + */ +export const visitorAddressField = "c-ip"; + +/** + * The delivered field names with the viewer's address left out. + * + * A delivery configured with these writes a record of requests. Every rollup + * except the visitor count reads the same columns it always did, and + * `RollupSummaries` over the resulting table computes no visitor count and + * needs no salt. + * + * ```typescript + * new CloudFrontLogDelivery(this, "RainlyticsDelivery", { + * distributionId: "E1EXAMPLE1234", + * logBucket: logs.bucket, + * fields: logFieldNamesWithoutAddress, + * }); + * ``` + * + * The raw store is immutable, so this decides what a day holds on the day it + * is delivered. Turning it on later leaves the addresses already written + * where they are, until the log bucket's expiry reaches them. + * `docs/visitors/` has both halves of that. + */ +export const logFieldNamesWithoutAddress: readonly string[] = + deliveredLogFieldNames.filter((name) => name !== visitorAddressField); + +/** + * The fields a visitor count is computed from. + * + * `visitor-identity.ts` hashes the address together with the user agent, so a + * field set missing either one counts nobody. The user agent is delivered for + * the bot filter every rollup applies, which means the address is the field + * this turns on. + */ +export const visitorCountFields: readonly string[] = [ + visitorAddressField, + "cs(User-Agent)", +]; + +/** + * The visitor-count fields a field set leaves out, in declared order. + * + * Both of them or neither. A delivery carrying the address and no user agent + * counts nobody in exactly the way one carrying neither does, and a caller + * told to add the field it already has would go round the same loop twice. + */ +export function missingVisitorCountFields( + fields: readonly string[], +): readonly string[] { + return visitorCountFields.filter((field) => !fields.includes(field)); +} + +/** Whether a field set carries what a visitor count is computed from. */ +export function countsVisitorsFrom(fields: readonly string[]): boolean { + return missingVisitorCountFields(fields).length === 0; +} + /** * The Glue column name a field is read back through, whichever format the * delivery writes. diff --git a/src/rollups.ts b/src/rollups.ts index 373c4ac..80078dd 100644 --- a/src/rollups.ts +++ b/src/rollups.ts @@ -283,6 +283,27 @@ export interface Rollup { readonly body: (request: RollupRequest) => string; } +/** + * One question with its visitor count off. + * + * A deployment over a table carrying no viewer address computes no visitor + * count, and `RollupSummaries` derives that for the questions Rainlytics + * ships. A site naming its questions instead says so through this. + * + * ```typescript + * new RollupSummaries(this, "RainlyticsSummaries", { + * table, + * workgroup, + * rollups: [withoutVisitorCount(pageviews), referrers], + * }); + * ``` + */ +export function withoutVisitorCount(rollup: Rollup): Rollup { + const { countsVisitors: _countsVisitors, ...rest } = rollup; + + return rest; +} + /** * A request with whatever it left out filled in. *