PagedIterable<T> 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-java/sdk/keyvault/azure-security-keyvault-certificates/src/main/java/com/azure/security/keyvault/certificates/CertificateClient.java

Lines 988 to 994 in 8409111

	public PagedIterable<CertificateProperties> listPropertiesOfCertificates(boolean includePending, Context context) {
	return new PagedIterable<>(
	maxResults -> mapCertificateItemPage(
	implClient.getCertificatesSinglePage(vaultUrl, maxResults, includePending, context)),
	(continuationToken, maxResults) -> mapCertificateItemPage(
	implClient.getCertificatesNextSinglePage(continuationToken, vaultUrl, context)));
	}

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.

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:

package com.example;

import com.azure.identity.AzureCliCredential;
import com.azure.core.util.polling.LongRunningOperationStatus;
import com.azure.core.util.polling.SyncPoller;
import com.azure.identity.DefaultAzureCredentialBuilder;
import com.azure.security.keyvault.certificates.CertificateClient;
import com.azure.security.keyvault.certificates.CertificateClientBuilder;
import com.azure.security.keyvault.certificates.models.CertificateContact;
import com.azure.security.keyvault.certificates.models.CertificateIssuer;
import com.azure.security.keyvault.certificates.models.CertificateOperation;
import com.azure.security.keyvault.certificates.models.CertificatePolicy;
import com.azure.security.keyvault.certificates.models.CertificateProperties;
import com.azure.security.keyvault.certificates.models.IssuerProperties;
import com.azure.security.keyvault.certificates.models.KeyVaultCertificate;
import com.azure.security.keyvault.certificates.models.KeyVaultCertificateWithPolicy;
import com.azure.core.util.Context;

import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
import com.azure.core.credential.TokenCredential;

public class HelloAzure {
  public static void main(String[] args) {

  // 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
    CertificateClient certificateClient = new CertificateClientBuilder()
            .vaultUrl("https://<keyvault-name>.vault.azure.net/")
            .credential(new DefaultAzureCredentialBuilder().build())
            .buildClient();

    System.out.println("Certificates in the key vault (includePending = false):");
    // Let's list all the certificates in the key vault.
    for (CertificateProperties certificate : certificateClient.listPropertiesOfCertificates()) {
        KeyVaultCertificate certificateWithAllProperties =
            certificateClient.getCertificateVersion(certificate.getName(), certificate.getVersion());

        System.out.println(certificateWithAllProperties.getProperties().getName());
    }

    System.out.println("Certificates in the key vault (includePending = true):");
    // Let's list all the certificates in the key vault.
    for (CertificateProperties certificate : certificateClient.listPropertiesOfCertificates(true, Context.NONE)) {
        KeyVaultCertificate certificateWithAllProperties =
            certificateClient.getCertificateVersion(certificate.getName(), certificate.getVersion());

        System.out.println(certificateWithAllProperties.getProperties().getName());
    }
  }
}

<?xml version="1.0" encoding="UTF-8"?>
<project xmlns="http://maven.apache.org/POM/4.0.0"
         xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
    <modelVersion>4.0.0</modelVersion>

    <groupId>com.example</groupId>
    <artifactId>hello-azure</artifactId>
    <version>1.0-SNAPSHOT</version>
    <packaging>jar</packaging>

    <properties>
        <maven.compiler.source>11</maven.compiler.source>
        <maven.compiler.target>11</maven.compiler.target>
    </properties>

    <dependencies>
        <!-- Azure Identity for authentication -->
        <dependency>
            <groupId>com.azure</groupId>
            <artifactId>azure-identity</artifactId>
            <version>1.14.0</version>
        </dependency>
        <dependency>
            <groupId>com.azure</groupId>
            <artifactId>azure-security-keyvault-certificates</artifactId>
            <version>4.7.0</version>
        </dependency>

        <!-- SLF4J Binding (required for logging) -->
        <dependency>
            <groupId>org.slf4j</groupId>
            <artifactId>slf4j-api</artifactId>
            <version>1.7.32</version>
        </dependency>
        <dependency>
            <groupId>org.slf4j</groupId>
            <artifactId>slf4j-simple</artifactId>
            <version>1.7.32</version>
        </dependency>
    </dependencies>
</project>

The issue is pervasive across all the PagedIterable<T> methods that follow this pattern within the KeyVault SDKs, but listPropertiesOfCertificates and listDeletedCertificates (along with maybe listRoleDefinitions in KeyVault Administration) 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 PagedIterable pattern, set the optional parameters appropriately in both the first and subsequent pages:

azure-sdk-for-java/sdk/tables/azure-data-tables/src/main/java/com/azure/data/tables/TableClient.java

Lines 1039 to 1046 in 8409111

	public PagedIterable<TableEntity> listEntities(ListEntitiesOptions options, Duration timeout, Context context) {
	Supplier<PagedIterable<TableEntity>> callable
	= () -> new PagedIterable<>(() -> listEntitiesFirstPage(context, options, TableEntity.class),
	token -> listEntitiesNextPage(token, context, options, TableEntity.class));
	return callIterableWithOptionalTimeout(callable, timeout, logger);
	}

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
Azure/azure-sdk-for-js#31803
#42988

[github-actions]

Thank you for your feedback. Tagging and routing to the team member best able to assist.