diff --git a/README.md b/README.md
index 1a461e3..f9e9e7b 100644
--- a/README.md
+++ b/README.md
@@ -388,6 +388,7 @@ This is the current list:
| |:white_check_mark:| PO036 | ServiceCallout Response element usage | The Response element, when present, should specify a text value and no attributes. |
| |:white_check_mark:| PO037 | DataCapture policy hygiene | Checks that a Capture should uses a Source of type request when the policy is attached to the Response flow, and other checks. |
| |:white_check_mark:| PO038 | KeyValueMapOperations policy hygiene | Checks that MapName or mapIdentifier is specified, and not both.|
+| |:white_check_mark:| PO039 | MessageLogging policy hygiene | Checks that ResourceType is not used, or is "api".|
| FaultRules | | | | |
| |:white_check_mark:| FR001 | No Condition on FaultRule | Use Condition elements on FaultRules, unless it is the fallback rule. |
| |:white_check_mark:| FR002 | DefaultFaultRule Structure | DefaultFaultRule should have only supported child elements, at most one AlwaysEnforce element, and at most one Condition element. |
diff --git a/lib/package/plugins/PO039-messageLogging-responseType.js b/lib/package/plugins/PO039-messageLogging-responseType.js
new file mode 100644
index 0000000..4aec269
--- /dev/null
+++ b/lib/package/plugins/PO039-messageLogging-responseType.js
@@ -0,0 +1,203 @@
+/*
+ Copyright 2019-2020,2024 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 lintUtil = require("../lintUtil.js"),
+ xpath = require("xpath"),
+ ruleId = lintUtil.getRuleId(),
+ debug = require("debug")("apigeelint:" + ruleId);
+
+const plugin = {
+ ruleId,
+ name: "Check MessageLogging / CloudLogging hygiene",
+ message:
+ "In the MessageLogging policy, the CloudLogging element should use correct hygiene.",
+ fatal: false,
+ severity: 1, // 1=warning, 2=error
+ nodeType: "Policy",
+ enabled: true,
+};
+
+const onPolicy = function (policy, cb) {
+ let hadWarning = false;
+ if (policy.getType() === "MessageLogging") {
+ debug(`found policy ${policy.getName()}`);
+ const clElements = xpath.select(
+ "/MessageLogging/CloudLogging",
+ policy.getElement(),
+ );
+ try {
+ if (clElements && clElements[0]) {
+ debug(`found ${clElements.length} response element(s)`);
+ if (clElements[1]) {
+ hadWarning = true;
+ policy.addMessage({
+ plugin,
+ message: "Policy has more than one CloudLogging element.",
+ severity: 2, // 1=warning, 2=error
+ line: clElements[1].lineNumber,
+ column: clElements[1].columnNumber,
+ });
+ }
+
+ // now check the zeroth element
+ const requiredChildren = ["LogName", "Message"];
+ const optionalChildren = ["Labels", "ResourceType"];
+ const validChildren = requiredChildren.concat(optionalChildren);
+ let found = {};
+
+ const children = xpath.select(`*`, clElements[0]);
+ children.forEach((child) => {
+ if (validChildren.includes(child.tagName)) {
+ if (found[child.tagName]) {
+ policy.addMessage({
+ plugin,
+ message: `Found more than one CloudLogging/${child.tagName} element.`,
+ severity: 2, // 1=warning, 2=error
+ line: child.lineNumber,
+ column: child.columnNumber,
+ });
+ } else {
+ found[child.tagName] = 1;
+ }
+ } else {
+ policy.addMessage({
+ plugin,
+ message: `Found more than one CloudLogging/${child.tagName} element.`,
+ severity: 2, // 1=warning, 2=error
+ line: child.lineNumber,
+ column: child.columnNumber,
+ });
+ }
+ });
+
+ requiredChildren.forEach((requiredElementName) => {
+ if (!found[requiredElementName]) {
+ hadWarning = true;
+ policy.addMessage({
+ plugin,
+ message: `Policy is missing a required Element: CloudLogging/${requiredElementName} element.`,
+ severity: 2, // 1=warning, 2=error
+ line: clElements[0].lineNumber,
+ column: clElements[0].columnNumber,
+ });
+ }
+ });
+
+ if (found.ResourceType) {
+ const rtypeElt = xpath.select(`ResourceType`, clElements[0])[0];
+ const textValue =
+ rtypeElt.childNodes &&
+ rtypeElt.childNodes[0] &&
+ rtypeElt.childNodes[0].nodeValue;
+ debug(`rtypeElt textValue '${textValue}'`);
+ if (!textValue || !textValue.trim()) {
+ hadWarning = true;
+ policy.addMessage({
+ plugin,
+ message: `ResourceType element is present but empty. Remove it or specify 'api'.`,
+ severity: 1, // 1=warning, 2=error
+ line: rtypeElt.lineNumber,
+ column: rtypeElt.columnNumber,
+ });
+ } else if (textValue != "api") {
+ hadWarning = true;
+ policy.addMessage({
+ plugin,
+ message: `The value '${textValue}' should not be used here. ResourceType should be 'api'`,
+ severity: 1, // 1=warning, 2=error
+ line: rtypeElt.lineNumber,
+ column: rtypeElt.columnNumber,
+ });
+ }
+ }
+
+ if (found.Labels) {
+ // do some checks here
+ const labelsElt = xpath.select(`Labels`, clElements[0])[0];
+ // the only allowed child of Labels is Label, and that should have just Key + Value
+ const labelsChildren = xpath.select(`*`, labelsElt);
+ labelsChildren.forEach((labelsChild) => {
+ if (labelsChild.tagName != "Label") {
+ hadWarning = true;
+ policy.addMessage({
+ plugin,
+ message: `Unsupported element '${labelsChild.tagName}'`,
+ severity: 2, // 1=warning, 2=error
+ line: labelsChild.lineNumber,
+ column: labelsChild.columnNumber,
+ });
+ } else {
+ // check each child
+ const validLabelChildren = ["Key", "Value"];
+ let found = {};
+ xpath.select(`*`, labelsChild).forEach((labelChild) => {
+ if (!validLabelChildren.includes(labelChild.tagName)) {
+ hadWarning = true;
+ policy.addMessage({
+ plugin,
+ message: `Unsupported element '${labelChild.tagName}'`,
+ severity: 2, // 1=warning, 2=error
+ line: labelChild.lineNumber,
+ column: labelChild.columnNumber,
+ });
+ } else {
+ if (found[labelChild.tagName]) {
+ policy.addMessage({
+ plugin,
+ message: `Found more than one CloudLogging/${labelChild.tagName} element.`,
+ severity: 2, // 1=warning, 2=error
+ line: labelChild.lineNumber,
+ column: labelChild.columnNumber,
+ });
+ } else {
+ found[labelChild.tagName] = 1;
+ }
+ }
+ });
+ validLabelChildren.forEach((requiredElementName) => {
+ if (!found[requiredElementName]) {
+ hadWarning = true;
+ policy.addMessage({
+ plugin,
+ message: `Label is missing a required Element: ${requiredElementName}.`,
+ severity: 2, // 1=warning, 2=error
+ line: labelsChild.lineNumber,
+ column: labelsChild.columnNumber,
+ });
+ }
+ });
+ }
+ });
+ }
+ }
+ } catch (e1) {
+ hadWarning = true;
+ policy.addMessage({
+ plugin,
+ message:
+ "Exception while examining CloudLogging element: " + e1.message,
+ });
+ }
+ }
+ if (typeof cb == "function") {
+ cb(null, hadWarning);
+ }
+};
+
+module.exports = {
+ plugin,
+ onPolicy,
+};
diff --git a/test/fixtures/resources/PO039/ML-test-invalid1.xml b/test/fixtures/resources/PO039/ML-test-invalid1.xml
new file mode 100644
index 0000000..9719465
--- /dev/null
+++ b/test/fixtures/resources/PO039/ML-test-invalid1.xml
@@ -0,0 +1,29 @@
+
+
+ projects/{organization.name}/logs/{proxylog}
+ {
+ "organization": "{organization.name}",
+ "environment": "{environment.name}",
+ "apiproxy": "{apiproxy.name}",
+ "revision": "{apiproxy.revision}",
+ "messageid": "{messageid}",
+ "url": "{proxy.url}"
+ }
+
+
+
+
+ gce_instance
+
+
diff --git a/test/fixtures/resources/PO039/ML-test-invalid2.xml b/test/fixtures/resources/PO039/ML-test-invalid2.xml
new file mode 100644
index 0000000..2043ea7
--- /dev/null
+++ b/test/fixtures/resources/PO039/ML-test-invalid2.xml
@@ -0,0 +1,29 @@
+
+
+ projects/{organization.name}/logs/{proxylog}
+ {
+ "organization": "{organization.name}",
+ "environment": "{environment.name}",
+ "apiproxy": "{apiproxy.name}",
+ "revision": "{apiproxy.revision}",
+ "messageid": "{messageid}",
+ "url": "{proxy.url}"
+ }
+
+
+
+
+ apigee.googleapis.com/Environment
+
+
diff --git a/test/fixtures/resources/PO039/ML-test-invalid3.xml b/test/fixtures/resources/PO039/ML-test-invalid3.xml
new file mode 100644
index 0000000..479f500
--- /dev/null
+++ b/test/fixtures/resources/PO039/ML-test-invalid3.xml
@@ -0,0 +1,29 @@
+
+
+ projects/{organization.name}/logs/{proxylog}
+ {
+ "organization": "{organization.name}",
+ "environment": "{environment.name}",
+ "apiproxy": "{apiproxy.name}",
+ "revision": "{apiproxy.revision}",
+ "messageid": "{messageid}",
+ "url": "{proxy.url}"
+ }
+
+
+
+
+
+
+
+
+
+
diff --git a/test/fixtures/resources/PO039/ML-test-valid1.xml b/test/fixtures/resources/PO039/ML-test-valid1.xml
new file mode 100644
index 0000000..083e1e4
--- /dev/null
+++ b/test/fixtures/resources/PO039/ML-test-valid1.xml
@@ -0,0 +1,27 @@
+
+
+ projects/{organization.name}/logs/{proxylog}
+ {
+ "organization": "{organization.name}",
+ "environment": "{environment.name}",
+ "apiproxy": "{apiproxy.name}",
+ "revision": "{apiproxy.revision}",
+ "messageid": "{messageid}",
+ "url": "{proxy.url}"
+ }
+
+
+ api
+
+
diff --git a/test/fixtures/resources/PO039/ML-test-valid2.xml b/test/fixtures/resources/PO039/ML-test-valid2.xml
new file mode 100644
index 0000000..57b639e
--- /dev/null
+++ b/test/fixtures/resources/PO039/ML-test-valid2.xml
@@ -0,0 +1,30 @@
+
+
+ projects/{organization.name}/logs/{proxylog}
+ {
+ "organization": "{organization.name}",
+ "environment": "{environment.name}",
+ "apiproxy": "{apiproxy.name}",
+ "revision": "{apiproxy.revision}",
+ "messageid": "{messageid}",
+ "url": "{proxy.url}"
+ }
+
+
+
+
+
+
diff --git a/test/fixtures/resources/PO039/ML-test-valid3.xml b/test/fixtures/resources/PO039/ML-test-valid3.xml
new file mode 100644
index 0000000..03885f5
--- /dev/null
+++ b/test/fixtures/resources/PO039/ML-test-valid3.xml
@@ -0,0 +1,29 @@
+
+
+ projects/{organization.name}/logs/{proxylog}
+ {
+ "organization": "{organization.name}",
+ "environment": "{environment.name}",
+ "apiproxy": "{apiproxy.name}",
+ "revision": "{apiproxy.revision}",
+ "messageid": "{messageid}",
+ "url": "{proxy.url}"
+ }
+
+
+
+
+
+
+
+
+
+
diff --git a/test/specs/PO039-test.js b/test/specs/PO039-test.js
new file mode 100644
index 0000000..e11fe83
--- /dev/null
+++ b/test/specs/PO039-test.js
@@ -0,0 +1,105 @@
+/*
+ Copyright 2019-2024 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 testID = "PO039",
+ assert = require("assert"),
+ fs = require("fs"),
+ path = require("path"),
+ bl = require("../../lib/package/bundleLinter.js"),
+ plugin = require(bl.resolvePlugin(testID)),
+ debug = require("debug")("apigeelint:" + testID),
+ Policy = require("../../lib/package/Policy.js"),
+ Dom = require("@xmldom/xmldom").DOMParser,
+ rootDir = path.resolve(__dirname, "../fixtures/resources/PO039");
+
+const test = (suffix, cb) => {
+ const filename = `ML-test-${suffix}.xml`;
+ it(`should correctly process ${filename}`, () => {
+ const fqfname = path.join(rootDir, filename),
+ policyXml = fs.readFileSync(fqfname, "utf-8"),
+ doc = new Dom().parseFromString(policyXml),
+ p = new Policy(doc.documentElement, this);
+
+ p.getElement = () => doc.documentElement;
+
+ //plugin.onBundle({ profile: "apigee" });
+
+ plugin.onPolicy(p, (e, foundIssues) => {
+ assert.equal(e, undefined, "should be undefined");
+ cb(p, foundIssues);
+ });
+ });
+};
+
+describe(`${testID} - MessageLogging RessourceType element`, () => {
+ // test all the valid cases
+ fs.readdirSync(rootDir)
+ .map((shortFileName) => {
+ let m = shortFileName.match("^.+-(valid.+)\\.xml$");
+ if (m) {
+ return m[1];
+ }
+ })
+ .filter((suffix) => suffix)
+ .forEach((suffix) => {
+ test(suffix, (p, foundIssues) => {
+ const messages = p.getReport().messages;
+ assert.ok(messages, "messages undefined");
+ debug(messages);
+ assert.equal(foundIssues, false);
+ });
+ });
+
+ test("invalid1", (p, foundIssues) => {
+ assert.equal(foundIssues, true);
+ const messages = p.getReport().messages;
+ assert.ok(messages, "messages undefined");
+ debug(messages);
+ assert.equal(messages.length, 1, "unexpected number of messages");
+ assert.ok(messages[0].message, "did not find message 0");
+ assert.equal(
+ messages[0].message,
+ "The value 'gce_instance' should not be used here. ResourceType should be 'api'",
+ );
+ });
+
+ test("invalid2", (p, foundIssues) => {
+ assert.equal(foundIssues, true);
+ const messages = p.getReport().messages;
+ assert.ok(messages, "messages undefined");
+ debug(messages);
+ assert.equal(messages.length, 1, "unexpected number of messages");
+ assert.ok(messages[0].message, "did not find message 0");
+ assert.equal(
+ messages[0].message,
+ "The value 'apigee.googleapis.com/Environment' should not be used here. ResourceType should be 'api'",
+ );
+ });
+
+ test("invalid3", (p, foundIssues) => {
+ assert.equal(foundIssues, true);
+ const messages = p.getReport().messages;
+ assert.ok(messages, "messages undefined");
+ debug(messages);
+ assert.equal(messages.length, 2, "unexpected number of messages");
+ assert.ok(messages[0].message, "did not find message 0");
+ assert.equal(messages[0].message, "Unsupported element 'NotKey'");
+ assert.equal(
+ messages[1].message,
+ "Label is missing a required Element: Key.",
+ );
+ });
+});