PagedAsyncIterableIterator KeyVault Certificate listPropertiesOfCertificates does not honor the includePending optional parameter after the first page

[ahsonkhan]

The call to fetch the first page sets the appropriate query parameters based on the input parameter value:

azure-sdk-for-js/sdk/keyvault/keyvault-certificates/src/index.ts

Lines 349 to 366 in 52c6f9d

	public listPropertiesOfCertificates(
	options: ListPropertiesOfCertificatesOptions = {},
	): PagedAsyncIterableIterator<CertificateProperties> {
	const iter = this.listPropertiesOfCertificatesAll(options);
	const result = {
	next() {
	return iter.next();
	},
	[Symbol.asyncIterator]() {
	return this;
	},
	byPage: (settings: PageSettings = {}) =>
	this.listPropertiesOfCertificatesPage(settings, options),
	};
	return result;
	}

But subsequent pages do not have that value set. That means, if the includePending param is set to true, it will not return all the certificates (including pending ones), if the pending certificate happens to be listed in a page other than the first.

azure-sdk-for-js/sdk/keyvault/keyvault-certificates/src/index.ts

Lines 286 to 306 in 52c6f9d

	const currentSetResponse = await tracingClient.withSpan(
	"CertificateClient.listPropertiesOfCertificatesPage",
	optionsComplete,
	(updatedOptions) => this.client.getCertificates(this.vaultUrl, updatedOptions),
	);
	continuationState.continuationToken = currentSetResponse.nextLink;
	if (currentSetResponse.value) {
	yield currentSetResponse.value.map(getPropertiesFromCertificateBundle, this);
	}
	}
	while (continuationState.continuationToken) {
	const currentSetResponse = await tracingClient.withSpan(
	"CertificateClient.listPropertiesOfCertificatesPage",
	options,
	(updatedOptions) =>
	this.client.getCertificatesNext(
	this.vaultUrl,
	continuationState.continuationToken!,
	updatedOptions,
	),
	);

Here's the swagger (not sure if this requires some fix to the swagger):
https://github.com/Azure/azure-rest-api-specs/blob/4a4acecea9901c29e19ba50f2d4cf65b20115b69/specification/keyvault/data-plane/Microsoft.KeyVault/stable/7.5/certificates.json#L30-L83

Sample repro:

const { CertificateClient } = require("@azure/keyvault-certificates");
const { DefaultAzureCredential } = require("@azure/identity");

async function main() {

  // Pre-req: Create 25 certificates first so a page is full (either through the portal or programmatically) 

  // Case 1: Create a certificate (either on the portal or programmatically) on the first page, and run this, right away.
  // Works as expected.
  // Case 2: Create a certificate (either on the portal or programmatically) on any subsequent page, and run this, right away.
  // Doesn't work as expected.

  // TODO: Set to your own KeyVault URL
  const url = "https://<keyvault-name>.vault.azure.net/";
  const credential = new DefaultAzureCredential();

  const client = new CertificateClient(url, credential);

  console.log("Certificates in the key vault (includePending = false):")
  for await (const certificate of client.listPropertiesOfCertificates()) {
    console.log(certificate.name);
  }

  console.log("Certificates in the key vault (includePending = true):")
  let pageCount = 0;
  for await (const page of client.listPropertiesOfCertificates({ includePending: true }).byPage()) {
    for (const certificate of page) {
      console.log(`Certificate from page ${pageCount}: `, certificate.name);
    }
    pageCount++;
  }
}

main().catch((error) => {
  console.error("An error occurred:", error);
  process.exit(1);
});

module.exports = { main };

The issue is pervasive across all the PagedAsyncIterableIterator methods that follow this pattern within the KeyVault SDKs, but listPropertiesOfCertificates and listDeletedCertificates seem to be the only ones that have optional parameters which are settable by the SDK methods (unlike maxResults) and hence have an actual behavioral bug here.

It's possible that some other service SDKs have similar concerns here, but newer SDKs PagedAsyncIterableIterator pattern, such as listRoleAssignments in Azure.Security.KeyVault.Administration might work correctly:

azure-sdk-for-js/sdk/tables/data-tables/src/TableClient.ts

Lines 495 to 510 in 52c6f9d

	while (result.continuationToken) {
	const optionsWithContinuation: InternalListTableEntitiesOptions = {
	...options,
	continuationToken: result.continuationToken,
	};
	result = await tracingClient.withSpan(
	"TableClient.listEntitiesPage",
	optionsWithContinuation,
	(updatedOptions, span) => {
	span.setAttribute("continuationToken", result.continuationToken);
	return this._listEntities<T>(tableName, updatedOptions);
	},
	);
	yield result;
	}

azure-sdk-for-js/sdk/keyvault/keyvault-admin/src/accessControlClient.ts

Lines 229 to 241 in 52c6f9d

	while (continuationState.continuationToken) {
	const currentSetResponse = await tracingClient.withSpan(
	"KeyVaultAccessControlClient.listRoleAssignmentsPage",
	options || {},
	async (updatedOptions) => {
	return this.client.roleAssignments.listForScopeNext(
	this.vaultUrl,
	roleScope,
	continuationState.continuationToken!,
	updatedOptions,
	);
	},
	);

Related issues in other languages:
Azure/azure-sdk-for-net#47202
Azure/azure-sdk-for-cpp#6235
Azure/azure-sdk-for-python#38589
Azure/azure-sdk-for-go#23772
#31803
Azure/azure-sdk-for-java#42988