runtime.Pager KeyVault Certificate NewListCertificatePropertiesPager 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-go/sdk/security/keyvault/azcertificates/client.go

Lines 742 to 768 in 3199096

	// NewListCertificatePropertiesPager - The GetCertificates operation returns the set of certificates resources in the specified
	// key vault. This operation requires the certificates/list permission.
	//
	// Generated from API version 7.5
	// - options - ListCertificatePropertiesOptions contains the optional parameters for the Client.NewListCertificatePropertiesPager
	// method.
	func (client *Client) NewListCertificatePropertiesPager(options *ListCertificatePropertiesOptions) *runtime.Pager[ListCertificatePropertiesResponse] {
	return runtime.NewPager(runtime.PagingHandler[ListCertificatePropertiesResponse]{
	More: func(page ListCertificatePropertiesResponse) bool {
	return page.NextLink != nil && len(*page.NextLink) > 0
	},
	Fetcher: func(ctx context.Context, page *ListCertificatePropertiesResponse) (ListCertificatePropertiesResponse, error) {
	nextLink := ""
	if page != nil {
	nextLink = *page.NextLink
	}
	resp, err := runtime.FetcherForNextLink(ctx, client.internal.Pipeline(), nextLink, func(ctx context.Context) (*policy.Request, error) {
	return client.listCertificatePropertiesCreateRequest(ctx, options)
	}, nil)
	if err != nil {
	return ListCertificatePropertiesResponse{}, err
	}
	return client.listCertificatePropertiesHandleResponse(resp)
	},
	Tracer: client.internal.Tracer(),
	})
	}

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-go/sdk/security/keyvault/azcertificates/options.go

Lines 76 to 81 in 3199096

	// ListCertificatePropertiesOptions contains the optional parameters for the Client.NewListCertificatePropertiesPager
	// method.
	type ListCertificatePropertiesOptions struct {
	// Specifies whether to include certificates which are not completely provisioned.
	IncludePending *bool
	}

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 main

import (
	"context"
	"fmt"

	"github.com/Azure/azure-sdk-for-go/sdk/azidentity"
	"github.com/Azure/azure-sdk-for-go/sdk/security/keyvault/azcertificates"
)

func 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.

	o := azidentity.AzureCLICredentialOptions{
		AdditionallyAllowedTenants: []string{"*"},
	}

	credential, err := azidentity.NewAzureCLICredential(&o)

        // TODO: Set to your own KeyVault URL
	client, err := azcertificates.NewClient("https://<keyvault-name>.vault.azure.net/", credential, nil)

	ctx := context.Background()

	fmt.Println("Certificates in the key vault (IncludePending = false):")
	listCertsPagerFalse := client.NewListCertificatePropertiesPager(nil)
	for listCertsPagerFalse.More() {
		page, _ := listCertsPagerFalse.NextPage(ctx)
		for _, cert := range page.Value {
			fmt.Println(*cert.ID)
		}
	}

	// Define a boolean value to pass to IncludePending (true)
	includePending := true
	fmt.Println("Certificates in the key vault (IncludePending = true):")
	listCertsPagerTrue := client.NewListCertificatePropertiesPager(&azcertificates.ListCertificatePropertiesOptions{
		IncludePending: &includePending,
	})
	for listCertsPagerTrue.More() {
		page, _ := listCertsPagerTrue.NextPage(ctx)
		for _, cert := range page.Value {
			fmt.Println(*cert.ID)
		}
	}
}

module example/hello

go 1.23.3

require (
	github.com/Azure/azure-sdk-for-go/sdk/azcore v1.16.0 // indirect
	github.com/Azure/azure-sdk-for-go/sdk/azidentity v1.8.0 // indirect
	github.com/Azure/azure-sdk-for-go/sdk/internal v1.10.0 // indirect
	github.com/Azure/azure-sdk-for-go/sdk/security/keyvault/azcertificates v1.3.0 // indirect
	github.com/Azure/azure-sdk-for-go/sdk/security/keyvault/internal v1.1.0 // indirect
	github.com/AzureAD/microsoft-authentication-library-for-go v1.3.1 // indirect
	github.com/golang-jwt/jwt/v5 v5.2.1 // indirect
	github.com/google/uuid v1.6.0 // indirect
	github.com/kylelemons/godebug v1.1.0 // indirect
	github.com/pkg/browser v0.0.0-20240102092130-5ac0b6a4141c // indirect
	golang.org/x/crypto v0.28.0 // indirect
	golang.org/x/net v0.30.0 // indirect
	golang.org/x/sys v0.26.0 // indirect
	golang.org/x/text v0.19.0 // indirect
)

The issue is pervasive across all the runtime.Pager methods that follow this pattern within the KeyVault SDKs, but NewListCertificatePropertiesPager and NewListDeletedCertificatePropertiesPager 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 runtime.Pager pattern, that don't use FetcherForNextLink might work correctly:

azure-sdk-for-go/sdk/data/aztables/client.go

Lines 180 to 183 in 3199096

	resp, err := t.client.QueryEntities(ctx, t.name, &generated.TableClientQueryEntitiesOptions{
	NextPartitionKey: partKey,
	NextRowKey: rowKey,
	}, listOptions.toQueryOptions())

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

[github-actions]

Thanks for the feedback! We are routing this to the appropriate team for follow-up. cc @jlichwa @RandalliLama @schaabs.

[github-actions]

Thanks for the feedback! We are routing this to the appropriate team for follow-up. cc @jlichwa @RandalliLama @schaabs.